-
Notifications
You must be signed in to change notification settings - Fork 14
[testing CI] Fix str encode in ractors #691
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
Open
jhawthorn
wants to merge
13
commits into
master
Choose a base branch
from
fix_str_encode_in_ractors
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use a managed `fast_transcoder_table` to cache results of src/dest encoding lookup. Without the cache, it could take a while because `transcode_search_path` does a BFS to get from src to dest encodings due to base encodings, aliases, etc. Since this managed table is a single-level table instead of 2 levels like `transcoder_table`, we don't need VM lock held and can use RCU. Make sure VM lock is NOT held when calling `load_transcoder_entry`, as that could call `rb_require_internal_silent`.
…e first Loading needs to be outside of VM lock.
Most of the time this should work fine, but if a native extension calls `setlocale(SOME_LOCALE)`, and that encoding is not loaded, it will load next time `rb_locale_encoding()` is called. This is called for methods on ENV like ENV#[]. Since these ENV methods acquire the VM lock, we need to make sure the encoding is loaded prior to locking it. Calling `setlocale()` like this is supported. The documentation for `rb_locale_encoding()` states: ``` * This is dynamic. If you change the process' locale by e.g. calling * `setlocale(3)`, that should also change the return value of this function. * * There is no official way for Ruby scripts to manipulate locales, though. rb_encoding *rb_locale_encoding(void); ```
This avoids taking the VM lock when doing "str".encode(Encoding::UTF_32), for example.
We want to avoid taking the VM lock at all on transcoding operations like `String#encode`, so now we have another managed table so we can avoid going through a two-level table that requires a VM lock. I think this is justified because looking up transcoder entries is done fairly often. Ractors are now quite fast when using `String#encode`.
Add managed_st_table functions and use them in transcode.c.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.