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

sync: reduce (*Map).Load penalty for Stores with new keys #21032

Open
bcmills opened this issue Jul 16, 2017 · 3 comments
Open

sync: reduce (*Map).Load penalty for Stores with new keys #21032

bcmills opened this issue Jul 16, 2017 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 16, 2017

In the Go 1.9 implementation of sync.Map, storing any new key in the map causes Load calls on all new keys to acquire a mutex until enough misses have occurred to promote the read-write map again.

That's probably fine for append-only maps (such as the sync.Map uses in the standard library), because it goes away in the steady state. However, it's problematic for use in caches, where there may be frequent Load and Store calls for keys that miss the cache.

I suspect we could substantially reduce the Load penalty by using a Bloom or HyperLogLog filter instead of a simple boolean to store the set of new keys: a Load that misses in the read-only map would still have more cache misses due to writes to the filter, but it would no longer block Store calls or contend with other Loads.

@bcmills
Copy link
Contributor Author

bcmills commented Jul 16, 2017

(@odeke-em and @RLH both made suggestions along these lines.)

@thepudds
Copy link
Contributor

thepudds commented Dec 3, 2024

For anyone following this issue, Go 1.24 has a new implementation of sync.Map (originally merged in CL 573956 as an internal HashTrieMap, with a more recent stack of CLs here).

That older CL 573956 includes the comment:

The map scales perfectly up to 24 cores for Loads, which is the maximum available parallelism on my machine.

which can be contrasted with this (from the opening comment of this issue here):

In the Go 1.9 implementation of sync.Map, storing any new key in the map causes Load calls on all new keys to acquire a mutex until enough misses have occurred to promote the read-write map again.

If people are curious about performance improvements, it could be helpful to test it out via benchmarks for their use cases, which is relatively easy via gotip:

$ go install golang.org/dl/gotip@latest
$ gotip download
$ gotip test -bench=.                    # use gotip as your go command

Some additional comments by @mknyszek in #21035 (comment) and #21035 (comment).

@mknyszek
Copy link
Contributor

mknyszek commented Dec 3, 2024

Thanks @thepudds. The new map's Loads are entirely independent of Store calls (a Store won't force any slow path, Load calls don't acquire any locks), so I suspect this issue may now be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Projects
None yet
Development

No branches or pull requests

6 participants