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
merged 6 commits into from
Feb 22, 2018

Conversation

kares
Copy link
Member

@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 9.2.0.0 milestone Feb 16, 2018
@headius
Copy link
Member

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.

@kares
Copy link
Member Author

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 kares force-pushed the anno-binder-refactor branch from 18fa186 to 4e70b67 Compare February 22, 2018 09:48
@kares kares merged commit 863f012 into jruby:master Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants