oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
2.98k stars 180 forks source link

Prepending a module to Integer disables many Inlined*Node #3546

Closed eregon closed 2 months ago

eregon commented 2 months ago

And for instance ActiveSupport does this: https://github.com/rails/rails/blob/d462fb54b459e222efc2e2480397af44a9c55361/activesupport/lib/active_support/core_ext/numeric/conversions.rb#L143 https://github.com/rails/rails/blob/d462fb54b459e222efc2e2480397af44a9c55361/activesupport/lib/active_support/core_ext/object/json.rb#L48-L50 (there is a question if that should be include instead of prepend in Rails, but we should try to handle prepend nicely anyway)

Because these do not redefine Integer#==, etc, we should avoid invalidating in that case. Since we have the fine-grained method invalidation, the idea is to replace ModuleFields#inlinedBuiltinsAssumptions by the assumptions in MethodEntry, as those are not invalidated on prepend but only if the method is overridden/removed/undefined. Specifically, we would still have ModuleFields#inlinedBuiltinsAssumptions but it would point to the initial Assumption in MethodEntry for the corresponding method, and there would be no need invalidate them explicitly as that's already done for MethodEntry.

An easy way to repro this is:

$ jt -u jvm-ce ruby --engine.TraceAssumptions -e 'Integer.prepend Module.new; 1 == 2'
...
[engine] assumption 'inlined Integer#>' invalidated installed code ...

There should be no 'inlined ...' assumptions invalidated, only Profiled Argument/Return Type, i.e. same output as without the prepend.

Found together with @nirvdrum.