-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
trie: parallel commit (alternative version) #30545
Conversation
e6591dd
to
5d09de1
Compare
Running this on |
5d09de1
to
cd6b35e
Compare
Here's the same:ish chart as @rjl493456442 provided for #30461 It's a bit odd -- the previous PR did showed a clear difference, but this PR does not. I have no idea why that is. |
trie: test which shows that the commit is erroneous trie: make parallel committer correct trie/trienode: better merge of set fix error in bench
6519663
to
c16a018
Compare
I will now switch it a bit, so that:
|
So, apparently, this made a difference. I don't know why, but now it does show a difference. The only meaningful change I can think of, is that I now set back the minumum mutations from So, new charts: On the left: So, now it's showing what I had expected from the start. Here's a zoom in on the last 30 minutes, again this PR on the left, ancestor PR on the right: |
This change makes the trie commit operation concurrent, if the number of changes exceed 100. Co-authored-by: stevemilk <wangpeculiar@gmail.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This change makes the trie commit operation concurrent, if the number of changes exceed 100. Co-authored-by: stevemilk <wangpeculiar@gmail.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Alternative to #30461
I've attempted to do it differently.
With this approach, I get
So the
9ms
->10ms
for single-thread, and4ms
for multi-threaded approach. The mem usage is only marginally higher than currentmaster
.