Reduce contention in heavy IO operations #7806
Merged
Conversation
This file contains 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
This moves the creation of a new selector outside of the sync section, to avoid blocking threads that are just trying to put a selector back when we are waiting to open a new one. This whole class could use some improvement using nonblocking collections, but this should help reduce contention for now. See jruby#7805.
headius
force-pushed
the
reduce_contention_selector_pool
branch
from
2c11bc1
to
bea00c4
Compare
|
I've pushed an additional commit that removes all locking from SelectorPool by using concurrent collections. If the contention is really due to locking in SelectorPool, this should definitely eliminate it. |
headius
changed the title
The data struct logic has used VariableTableManager and internal variable logic for years, which is concurrency-safe. There's no need for these to be synchronized. Note that this code is not used many places, but notably all forms of IO#each (like File#each, Socket#each) use it to support the ARGF feature of Ruby, which is shared across threads. This showed up as a point of contention after removing SelectorPool's excess locking in jruby#7806, meant to address contention reported by LogStash users in jruby#7805.
|
Additional commit to remove synchronization from RubyBasicObject.dataGetStruct/dataWrapStruct because they should not need it (they utilize VariableTableManager which is concurrency-safe). |
The various forms of puts at the very least use the two-argument form, which was falling through the varargs path and allocating argument arrays. I add three-argument form for completeness.
headius
added a commit
to headius/jruby
that referenced
this pull request
May 30, 2023
The data struct logic has used VariableTableManager and internal variable logic for years, which is concurrency-safe. There's no need for these to be synchronized. Note that this code is not used many places, but notably all forms of IO#each (like File#each, Socket#each) use it to support the ARGF feature of Ruby, which is shared across threads. This showed up as a point of contention after removing SelectorPool's excess locking in jruby#7806, meant to address contention reported by LogStash users in jruby#7805.
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.
This moves the creation of a new selector outside of the sync section, to avoid blocking threads that are just trying to put a selector back when we are waiting to open a new one.
This whole class could use some improvement using nonblocking collections, but this should help reduce contention for now.
Edit: This has expanded to remove additional areas of contention.
See #7805.