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

refactor annotation-binder generated populators #5050

merged 6 commits into from Feb 22, 2018


Copy link

@kares kares commented Feb 16, 2018

accounts for a slight speed-up - the populator won't execute much 'redundant' code

this isn't the main issue with boot (compared to invokers) but still deserves a hand-craft

@kares kares added the internal label Feb 16, 2018
@kares kares added this to the JRuby milestone Feb 16, 2018
Copy link

@headius headius commented Feb 20, 2018

This all seems fine, especially if it's slightly faster.

Note the fix I made for legacy exts that have "compat" set to 1.8.

FWIW I introduced the "packed" paths to try to shrink the amount of code we generate for populators and to reduce the amount of object churn booting up core classes. However I realize now that packing them doesn't reduce total strings since we still have to parse the names out, and may make things worse since they aren't coming from constant pool anymore.


Copy link
Member Author

@kares kares commented Feb 21, 2018

thanks for the review.
exactly not re-using the string constants, which are already there anyway, seemed counter-productive.
will resolve/rebase with that 1_8 compat fix and merge.


kares added 6 commits Feb 22, 2018
from : 
MethodIndex.addMethodReadFieldsPacked(1023, "instance_eval;instance_eval;instance_eval;instance_eval;instance_exec");
``` :
MethodIndex.addMethodReadFieldsPacked(1023, "instance_eval;instance_exec");
(this avoids method lookups for aliases during boot)
@kares kares force-pushed the anno-binder-refactor branch from 18fa186 to 4e70b67 Feb 22, 2018
@kares kares merged commit 863f012 into jruby:master Feb 22, 2018
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants