triedb/pathdb, eth: use double-buffer mechanism in pathdb

[rjl493456442]

Previously, PathDB used a single buffer to aggregate database writes, which needed to be flushed atomically. However, flushing large amounts of data (e.g., 256MB) caused significant overhead, often blocking the system for around 3 seconds during the flush.

To mitigate this overhead and reduce performance spikes, a double-buffer mechanism is introduced. When the active buffer fills up, it is marked as frozen and a background flushing process is triggered. Meanwhile, a new buffer is allocated for incoming writes, allowing operations to continue uninterrupted.

This approach reduces system blocking times and provides flexibility in adjusting buffer parameters for improved performance.

TODO:

• release the content in the frozen buffer after flushing

[holiman]

All in all, this looks promising, I suspect this could help quite a bit

[holiman]

Note: at this point, mutations were already applied on the clean, i.e, dl.cleans cache. That happened during writeNodes. I've tried to figure out if that is a problem, but come to the conclusion that it's fine, but just wanted to raise it so you can also give it a think.

Regarding "flush all mutations in a single batch" -- is that important only because of crash-safety, or some other more subtle reason?

[holiman]

How about this
in disklayer.go, function node(), we lookup a node. Order:

1. buffer
2. frozen
3. cleans
4. database
   • And if found, write to cleans

	if dl.cleans != nil && len(blob) > 0 {
		dl.cleans.Set(key, blob)
		cleanWriteMeter.Mark(int64(len(blob)))
	}

I'm trying to think of a case where this write-to-cleans conflicts with the write-to-cleans in the background committer writeNodes method.

[rjl493456442]

• if it's found in buffer/frozen => return and no interaction with cache
• if it's found in cache => return
• if it's found in disk (it implicitly means the item is not in these places above, even the item is marked as deleted, it will still be caught in buffer/frozen/cache), load it from db and add it into the cache

so, no conflict should happen

But i have to say it's a really good point, i haven't thought about it

[rjl493456442]

Regarding "flush all mutations in a single batch" -- is that important only because of crash-safety, or some other more subtle reason?

Only because of crash-safety

[holiman]

LGTM, would be interesting to see some performance-charts. This PR needs some runtime before merging, IMO

[rjl493456442]

For sure, it’s not a please-merge-it pull request, it will be twisted a bit and have a full performance impact inspection Thanks and Best regards Gary rong Martin HS ***@***.***>于2024年9月23日 周一下午2:49写道：

…

***@***.**** approved this pull request. LGTM, would be interesting to see some performance-charts. This PR needs some runtime before merging, IMO — Reply to this email directly, view it on GitHub <#30464 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNO6OOSVOX3WRFPVBR6AJ3ZX62XNAVCNFSM6AAAAABOPFBYCCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMRRGI2DKNRVGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[joey0612]

Referenced #28471 ？

[rjl493456442]

The triedb commit is improved as expected, but the EVM execution is constantly slower, with unknown reasons.

[waynercheung]

Referenced #28471 ？

I have the same question, is this PR referenced #28471 ？

[rjl493456442]

@joey0612 @waynercheung

No, double-buffering has been part of my design since day one when I first implemented the path database.
I even had a prototype a few years ago. However, it hasn’t been merged yet for several reasons:
(a) the overall performance improvement was not significant; and (b) after flushing the write buffer into the database, subsequent database reads became 2–3× slower, which in turn slowed down the entire chain progression.

However, with the fix to block prefetcher, the state read has been significantly improved. And I want to pile
up this change on top one more time.

cockroachdb/pebble#4109

[MariusVanDerWijden]

LGTM

[MariusVanDerWijden]

This can never happen, even if the flushing takes a long time to do, right? Because we are always rotating out the buffers

[rjl493456442]

Exactly. The buffer is supposed to be flushed for one time.