Is overriding a method in a subclass considered as a breaking change or not?
- In [Bug #15589], I'm going to make
Integer#zero?faster (comparable to
== 0) on JIT by making sure the method is inlinable by MJIT.
Integer#zero?is not defined and it's implemented as
Numeric#zero?'s method definition handles not only
rb_funcallis used for
Complex, we can't predict what'd happen there and it prevents
Numeric#zero?from being inlined by MJIT. Moreover, reducing inlined code size is always good for JIT-ed code. So specializing
Integer#zero?will be beneficial for MJIT's inlining regardless of
- So I'd like to define
Integer#zero?for optimizing JIT-ed code of
Theoretically, overriding a method in a subclass in Ruby core might change an existing application's behavior (If a user overwrote
Numeric#zero? and then Ruby core overwrote
Integer#zero?, the user's definition would get suddenly ignored for Integer).
How should we decide whether the change is acceptable or not? Some possible ideas:
- Search all gems using gem-codesearch and check if there's any code to be broken.
Integer#zero?'s case I found this code but it's not gonna be broken by overriding
zero?method definitions seemed for other classes, but I can double check every occurrence if this will be the measure.
- Discuss and make a decision on case-by-case basis.
Integer#zero?'s case we'll discuss whether overriding
Integer#zero?is acceptable or not.
Updated by Eregon (Benoit Daloze) over 1 year ago
Could MJIT add its own check if called on a Fixnum, then do the inline fast-path logic, otherwise fallback to a call to the original definition?
One would still need to check that Numeric#zero? is not redefined, but in all cases it's needed to check if redefined (whether it's Numeric#zero? or Integer#zero?).
That would work transparently without adding a new method.
Somewhat similar logic in TruffleRuby for
nil?, etc, handling a subset of inputs, and rewriting as a call if inputs don't match that:
I think we should avoid many new methods on e.g. Integer/Array duplicating Numeric/Enumerable, but rather have generic tricks to be able to specialize on common cases.
Updated by marcandre (Marc-Andre Lafortune) over 1 year ago
I imagine that it is fine as long as it's not done in a patch release and it appears in the
Array has had some methods specialized for optimization purposes (
minmax in 2.7,
min/max/sum in 2.4, ...)
I doubt there are many valid reasons to redefine
Numeric#zero? such that
Integer#zero? wouldn't behave as expected.
Updated by k0kubun (Takashi Kokubun) over 1 year ago
Could MJIT add its own check if called on a Fixnum
You're right, that thing is technically not difficult. What I'm trying to do is to avoid adding MJIT-only method definition (like fast-path logic used only for MJIT) or (#ifdef) branches so that we do not cause bugs by discrepancy between VM and JIT and we do not need to maintain lots of copy-pasted code for MJIT.
I think we should avoid many new methods on e.g. Integer/Array duplicating Numeric/Enumerable
My assumption is that we already have many methods on
Integer rather than
Numeric, and this discussion is for minor leftovers which are inconsistent with other major methods. When such Numeric methods have branches for Integer, Complex, and Rational, removing a branch for Integer from Numeric and adding it as an Integer method don't cause a lot of code duplication at least.
rather have generic tricks to be able to specialize on common cases.
That'd be nice, but because I've already spent a lot of time for considering options I can easily think of, please describe your idea with concrete interface which is used to maintain it.
What kind of code in numeric.c or numeric.rb do you imagine to write for having Integer fast path for
num_zero_p? Doesn't it create a path or code duplication which is never used (or tested) on VM? If not, doesn't it complicate the original method definition? (Well, I'm not saying we should never do such things, but those are what I tried to maintain when choosing the current proposal.)
Another idea brought by ko1 in case overriding it is considered a breaking change is that we undef
Numeric#zero? is redefined. That's another vector for complication, but at least this technique could be also useful for VM and it's not too MJIT-specific.