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

1.9 Concurrent Maps #38

Closed
siennathesane opened this issue Jun 27, 2017 · 5 comments
Closed

1.9 Concurrent Maps #38

siennathesane opened this issue Jun 27, 2017 · 5 comments

Comments

@siennathesane
Copy link
Collaborator

Was looking at tip for 1.9, there is a new feature called Concurrent Maps, wanted to start a discussion around adding support for concurrent cache ops.

Not sure on the performance of it, but the design seems fairly solid. Looks like it uses a lot of unsafe.Pointer types and atomic ops, so it might be fairly fast.

@janisz
Copy link
Collaborator

janisz commented Jun 27, 2017

Looks interesting. We give it a shot and try to replace current maps and locks with concurrent map. Still we need to synchronize writes to array but reads could be faster.

@siennathesane
Copy link
Collaborator Author

Yeah, I think reads could be faster, but I think the writes will be about the same, maybe ±5% since the current implementation is so highly optimized. My thoughts on this would be to try replace the base map[uint64]uint32 implementation and see what happens. I think that and removing the explicit locking would be the only change for testing this.

If it's close in performance (±10%), it might also be useful as a secondary API instead. Something like concurrentCache := bigcache.NewBigCache(bigcache.Config{Concurrent: true}), where concurrentCache utilises the concurrent map feature, but keeps the interface the same. That being said, it would create a dependency on 1.9+, so that use case would have to be addressed.

My time for the next month is fairly limited since I'm moving, and 1.9 doesn't debut until August. I'll try and fiddle with it over the next month and see what I can come up with.

@janisz
Copy link
Collaborator

janisz commented Oct 10, 2017

#49 makes possible to do this task pretty easy. Just copy implementation of shard.go to new file (e.g., shard_concurent_map.go) and replace maps and locks with concurrent maps. Finally add build tags on both files: // +build !go1.9 and // +build go1.9. This will make use of concurrent hash map default in go 1.9. If the performance does not increase we can just add optional build tag to enable concurrent hash map instead of regular one.

@siennathesane
Copy link
Collaborator Author

Perfect. I will work on it in the next week or so.

@siennathesane
Copy link
Collaborator Author

Okay, this is actually going to be a big change as concurrent maps do not support indexing. So this will take awhile.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants