Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
matejsp wants to merge 1 commit into quickfixgo:main
base: main
Choose a base branch
Loading
from matejsp:remove-locking-for-fieldmap

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

kcross-ctoken reacted with thumbs up emoji
@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
Copy link
Contributor

Does this demonstrate improved performance on the ParseMessage benchmark?

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.

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.

ackleymi reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /