-
Notifications
You must be signed in to change notification settings - Fork 322
Replace locking fieldmap with concurrent safe haxmap #685
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
Conversation
08a1bc5 to
6106d2d
Compare
6106d2d to
22e752b
Compare
Does this demonstrate improved performance on the ParseMessage benchmark?
kcross-ctoken
commented
Jan 9, 2025
Its a lot simpler so it would save on maintenance.
Just be weary that sometimes the no-contention mechanisms mean its eventually consistent (buffered writes or delayed writes) which might pose problems with direct set and get code.
I just benchmarked and it seems that id does not work better. I suspect the issue is with haxmap doing initial allocation that is more slow while working on existing haxmap is a lot faster. But since we create lots of haxmaps it is actually not faster.
I tested the performance with and without rwmutex:
without rwmutex:
BenchmarkParseMessage-16 2524971 480.8 ns/op 20 B/op 2 allocs/op
with rwmutext:
BenchmarkParseMessage-16 2283403 528.4 ns/op 20 B/op 2 allocs/op
with haxmap:
BenchmarkParseMessage-16 843738 1353 ns/op 1536 B/op 34 allocs/op
The issue that we actually had with parsing is not related to rwmutext but to repeating_groups because they are parsed on each get and it's taking 30% of our processing:
imageI will do another PR to try to solve this.
I have replaced the locking of fieldmap with haxmap (A lightning fast concurrent hashmap):
https://github.com/alphadose/haxmap