-
-
Notifications
You must be signed in to change notification settings - Fork 230
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
chore: Replace sync.Mutex with sync.Map #1197
base: main
Are you sure you want to change the base?
Conversation
I think this might perform slightly better but I wonder if we could write a benchmark |
IMO it's a hard case to benchmark bc it requires a lot of concurrency and we cannot use gh actions for it |
Another option is to use |
To be honest without evidence this being anyhow better I would not proceed with the change. |
Added the benchmark |
So the benchmark shows this is indeed faster for the tests. The question remains about what happens with real concurrency, but I think it will only behave better because there is no full blocking. @jcchavezs Do you want to make benchmakrs with |
ping @jcchavezs |
I will do that tomorrow. |
LGTM |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1197 +/- ##
==========================================
- Coverage 81.66% 81.66% -0.01%
==========================================
Files 168 168
Lines 9655 9652 -3
==========================================
- Hits 7885 7882 -3
Misses 1519 1519
Partials 251 251
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Sync.Map provides a key level locking to allow concurrent key writes.
Make sure that you've checked the boxes below before you submit PR:
Thanks for your contribution ❤️
Benchmarks based on mutex vs sync.Map
sync.Mutex
sync.Map