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

Avoid fast-path IO writes when IO has ext enc #759

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

headius
Copy link
Contributor

@headius headius commented Mar 10, 2025

This works around a bug in JRuby's IOOutputStream logic whereby an IO with an external encoding will always fail to write incoming bytes. We use base logic to detect if the target object is a "real IO" and if so and it has an external encoding, we drop the realIO and. This causes the rest of IOOutputStream to avoid the fast path and always use dyncall logic with RubyString wrappers.

This works around the issue in jruby/jruby#8682.

This should be temporary, since it will definitely degrade direct writes to such IO objects, but a longer-term fix for the encoding issues spelled out in jruby/jruby#8682 will need to come first, or else json will have to be modified to not use IOOutputStream at all.

This works around a bug in JRuby's IOOutputStream logic whereby an
IO with an external encoding will always fail to write incoming
bytes. We use base logic to detect if the target object is a "real
IO" and if so and it has an external encoding, we drop the realIO
and. This causes the rest of IOOutputStream to avoid the fast path
and always use dyncall logic with RubyString wrappers.

This works around the issue in jruby/jruby#8682.

This should be temporary, since it will definitely degrade direct
writes to such IO objects, but a longer-term fix for the encoding
issues spelled out in jruby/jruby#8682 will need to come first,
or else json will have to be modified to not use IOOutputStream at
all.
@headius
Copy link
Contributor Author

headius commented Mar 10, 2025

The root issue this works around is technically jruby/jruby#6588 but this temporary fix will address the failing rdoc test from jruby/jruby#8682 and allow the test to be restored.

@headius
Copy link
Contributor Author

headius commented Mar 10, 2025

#760 details the ongoing work needed after this workaround.

@headius headius merged commit c079793 into ruby:master Mar 10, 2025
33 checks passed
@headius headius deleted the patch_iooutputstream_encoding branch March 10, 2025 17:36
@headius
Copy link
Contributor Author

headius commented Mar 10, 2025

@byroot @nobu When this is released, the rdoc test disabled for jruby/jruby#8682 can be restored by either getting a new JRuby out or by installing latest json gem.

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.

1 participant