Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
eregon opened this issue Apr 22, 2024 · 0 comments
Closed

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

eregon opened this issue Apr 22, 2024 · 0 comments

Comments

@eregon
Copy link
Member

eregon commented Apr 22, 2024

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.

@eregon eregon changed the title Prepending a module to Integer disbales many Inlined*Node Prepending a module to Integer disables many Inlined*Node Apr 22, 2024
@andrykonchin andrykonchin self-assigned this Apr 22, 2024
@eregon eregon self-assigned this Apr 23, 2024
graalvmbot pushed a commit that referenced this issue Apr 23, 2024
…ill hold

* This does not invalidate on prepend but only when redefining/overriding that specific method.
* Fixes #3546
graalvmbot pushed a commit that referenced this issue Apr 23, 2024
…ill hold

* This does not invalidate on prepend but only when redefining/overriding that specific method.
* Fixes #3546
@eregon eregon added this to the 24.1.0 Release (Sep 17, 2024) milestone Apr 23, 2024
graalvmbot pushed a commit that referenced this issue Apr 23, 2024
…ill hold

* This does not invalidate on prepend but only when redefining/overriding that specific method.
* Fixes #3546
graalvmbot pushed a commit that referenced this issue Apr 25, 2024
…ill hold

* This does not invalidate on prepend but only when redefining/overriding that specific method.
* Fixes #3546
graalvmbot pushed a commit that referenced this issue Apr 25, 2024
…ill hold

* This does not invalidate on prepend but only when redefining/overriding that specific method.
* Fixes #3546
@eregon eregon moved this to Done in TruffleRuby Roadmap Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants