[chore] processor/deltatocumulative: remove global mutex #37682
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
|
cc @ArthurSens |
sh0rez
force-pushed
the
lockfree
branch
2 times, most recently
from
d9b2c75 to
bc276be
Compare
ArthurSens
reviewed
Feb 10, 2025
ArthurSens
reviewed
Feb 14, 2025
There was a problem hiding this comment.
Did another pass here. I wonder if we could relax usage of generics, it feels like it's getting out of hand.
Why do we need Limited, Count and Sized implementations if we're never using them alone? There's one single usage of those maps and we're chaining them all together. Wouldn't it be more readable if we have one single generic implementation of this map?
sh0rez
force-pushed
the
lockfree
branch
2 times, most recently
from
be191cc to
554eb31
Compare
ArthurSens
reviewed
Feb 20, 2025
sh0rez
changed the title
ArthurSens
approved these changes
Feb 21, 2025
ArthurSens
reviewed
Feb 21, 2025
sh0rez
force-pushed
the
lockfree
branch
2 times, most recently
from
abf6d30 to
ba6f401
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
ArthurSens
added
the
ready to merge
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.
Description
Before, deltatocumulative acquired a
sync.Mutexon eachConsumeMetricsinvocation. This inherently does not scale well.To fix, I ran microbenchmarks of different threadsafe hashmap approaches and value types (see
internal/research/tracking).This implements the first finding, switching to
xsync.MapOffor stream tracking and wrapping the individual datapoints into mutexes, to allow for higher parallelism.Link to tracking issue
none
Testing
Existing test continue to pass.
A new
TestStalenesswas added using experimental Go 1.24testing/synctest, which for now can only be run manually: