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

Optimizations and cleanup for the JRuby extension #708

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

headius
Copy link
Contributor

@headius headius commented Nov 21, 2024

No description provided.

This improves performance by having fewer dynamic writes.
This eliminates the context field on ParserSession,
ByteListTranscoder, and Generator.Session as well as accesses of
the runtime through it. These runtime structures should generally
be passed on the stack rather than being stored into long-lived
objects, even if they may be GCed quickly.
@headius headius changed the title Jruby optz Optimizations and cleanup for the JRuby extension Nov 21, 2024
It's more efficient to compare the Encoding object and for the
rare cases where we need to transcode to UTF-8 just look it up
again.
Rarely used, mostly at boot, and probably just as quick to look up
the constant as go through the weak reference.
Previous logic would only pass the string through if its encoding
was exactly UTF-8. New logic passes through UTF-8 and US-ASCII and
handles other encodings like CRuby.
Using an output stream here is just added overhead compared to
writing directly to a ByteList. Instead, we move the OutputStream
logic into StringEncoder and restore direct ByteList writing to
StringDecoder.
@headius headius marked this pull request as ready for review November 22, 2024 21:07
@headius
Copy link
Contributor Author

headius commented Nov 22, 2024

This is done and ready for review. @byroot let me know if you have any concerns.

@byroot
Copy link
Member

byroot commented Nov 22, 2024

I don't have much Java expertise, so feel free to merge if you're ready.

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