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

Replace locking fieldmap with concurrent safe haxmap #685

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matejsp
Copy link

@matejsp matejsp commented Dec 22, 2024

I have replaced the locking of fieldmap with haxmap (A lightning fast concurrent hashmap):
https://github.com/alphadose/haxmap

@matejsp matejsp force-pushed the remove-locking-for-fieldmap branch from 08a1bc5 to 6106d2d Compare December 22, 2024 20:46
@matejsp matejsp force-pushed the remove-locking-for-fieldmap branch from 6106d2d to 22e752b Compare December 23, 2024 08:17
@michaelwilner
Copy link
Contributor

Does this demonstrate improved performance on the ParseMessage benchmark?

@kcross-ctoken
Copy link

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.

@matejsp
Copy link
Author

matejsp commented Jan 9, 2025

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:

image

I will do another PR to try to solve this.

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

Successfully merging this pull request may close these issues.

3 participants