internal/core/runtime: replace slice with sync.Map#4262
Open
jonjohnsonjr wants to merge 1 commit intocue-lang:masterfrom
Open
internal/core/runtime: replace slice with sync.Map#4262jonjohnsonjr wants to merge 1 commit intocue-lang:masterfrom
jonjohnsonjr wants to merge 1 commit intocue-lang:masterfrom
Conversation
We have a workload that makes many calls to cue.Value.Path(), which spends most of its time inside IndexToString's RWMutex.Rlock() call. This change attempts to optimize that by replacing the `labels` slice with a `sync.Map`. From `sync.Map`'s godoc: > The Map type is optimized for two common use cases: > (1) when the entry for a given key is only ever written once but read > many times, as in caches that only grow, ... > In these two cases, use of a Map may significantly reduce lock > contention compared to a Go map paired with a separate Mutex or RWMutex. I believe this maps exactly to what IndexToString is doing, and indeed we spend about 50% less time inside IndexToString after this change. Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Member
|
Assigning to @rogpeppe given that he's been looking at concurrency issues with Marcel, and he's also been chatting with you recently :) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
We have a workload that makes many calls to cue.Value.Path(), which spends most of its time inside IndexToString's RWMutex.Rlock() call.
This change attempts to optimize that by replacing the
labelsslice with async.Map.From
sync.Map's godoc:I believe this maps exactly to what IndexToString is doing, and indeed we spend about 50% less time inside IndexToString after this change.