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

Improve efficiency of the scan for dirty nodes #941

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

HerbertJordan
Copy link
Collaborator

@HerbertJordan HerbertJordan commented Jun 28, 2024

This PR applies some performance improvement on the scan for dirty nodes during flush operations. The main change is to track the dirty-state of nodes using atomic operations to facilitate checking for dirty nodes without the need of acquiring node-access permissions.

This reduces the time it takes to search for dirty nodes by more than 60%:

goos: linux
goarch: amd64
pkg: github.com/Fantom-foundation/Carmen/go/database/mpt
cpu: Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz
                                              │  before.log  │              after.log               │
                                              │    sec/op    │    sec/op     vs base                │
NodeFlusher_CollectDirtyNodes/size=1000-4       33.796µ ± 4%   5.372µ ±  3%  -84.10% (p=0.000 n=10)
NodeFlusher_CollectDirtyNodes/size=10000-4      335.45µ ± 3%   54.32µ ±  7%  -83.81% (p=0.000 n=10)
NodeFlusher_CollectDirtyNodes/size=100000-4      3.891m ± 1%   1.462m ±  3%  -62.41% (p=0.000 n=10)
NodeFlusher_CollectDirtyNodes/size=1000000-4     39.50m ± 3%   14.70m ±  2%  -62.78% (p=0.000 n=10)
NodeFlusher_CollectDirtyNodes/size=10000000-4    399.0m ± 4%   142.6m ± 30%  -64.25% (p=0.000 n=10)
geomean                                          3.702m        978.0µ        -73.58%

Notes for Reviewers

By introducing atomic operations on the clean flag in the nodeBase assignment operations on the node level like the one done in the Forest implementation (e.g. *write.Get().(*AccountNode) = *node) introduce a race condition since the reset of the clean flag is not atomic in this case. Thus, a new Reset function needed to be introduced, performing the clean flag reset using an atomic operation.

@HerbertJordan HerbertJordan force-pushed the herbert/tune_dirty_node_scans branch from 4d4e83b to 37a8a05 Compare June 29, 2024 16:39
@HerbertJordan HerbertJordan requested a review from kjezek June 30, 2024 14:05
@HerbertJordan HerbertJordan marked this pull request as ready for review June 30, 2024 14:05
Copy link
Collaborator

@kjezek kjezek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is not used atomic.Bool. Otherwise it looks good, thanks

go/database/mpt/nodes.go Show resolved Hide resolved
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.

2 participants