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

Make all service loading static #1816

Merged
merged 1 commit into from
Feb 8, 2025
Merged

Conversation

gbrail
Copy link
Collaborator

@gbrail gbrail commented Feb 5, 2025

This prevents the large performance regression of loading these classes from the classpath every time they are needed, and should cover existing use cases since all these features are designed to be configured statically by deciding which Rhino modules to use.

@rbri I didn't see a big regression in standard benchmarks like the V8 benchmarks but I could imagine that in a use case with a lot of reflection that things could be much worse. This should fix it, please take a look!

This prevents the large performance regression of loading these classes
from the classpath every time they are needed, and should cover existing
use cases since all these features are designed to be configured
statically by deciding which Rhino modules to use.
@rbri
Copy link
Collaborator

rbri commented Feb 5, 2025

thanks, @gbrail will have a look at this during the next days...

@rbri
Copy link
Collaborator

rbri commented Feb 6, 2025

@gbrail have done a small test, looks like your change brings back the old performance.
+1 for merging

@gbrail
Copy link
Collaborator Author

gbrail commented Feb 8, 2025

Yes, this one is a pretty simple one. Thanks for checking it out!

@gbrail gbrail merged commit ea518c2 into mozilla:master Feb 8, 2025
3 checks passed
@gbrail gbrail deleted the greg-loaders branch February 8, 2025 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants