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

core/state: remove slot dirtyness if it's set back to origin value #29731

Merged
merged 2 commits into from
May 10, 2024

Conversation

rjl493456442
Copy link
Member

This pull request reworks the dirty storage logic a bit, by handling one more corner case for revoking dirtiness.

The dirty marker of a storage slot will be revoked in these following two cases:

  • the initial mutation of the slot inside of the current transaction is reverted
  • the storage slot is set back to its original value

In these two cases, the storage slot shouldn't be regarded as dirty anymore.

@rjl493456442 rjl493456442 added this to the 1.14.2 milestone May 8, 2024
@rjl493456442 rjl493456442 force-pushed the filter-noop-storage-update branch from 851912f to 5005f21 Compare May 8, 2024 07:14
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM, lets see what benchmarkers say

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

We could fix it here, but with the kind of journal that we have now, where we just linerarly pile events on events, it's not "free".
Might make sense to either apply this later on, in a non-linear journal, OR to do this check later on, during commit (?)

core/state/state_object.go Outdated Show resolved Hide resolved
core/state/state_object.go Outdated Show resolved Hide resolved
@rjl493456442
Copy link
Member Author

I would prefer to fix it there, namely the storage we finalize are truly dirty. It's a cleaner solution that we can make sure in each transaction frame the storage changes are correct and then we only need to care about the logic for merging the storage changes between different frames.

@rjl493456442
Copy link
Member Author

Bench07: PR
Bench08: Master

Overall performance is basically same.

截屏2024-05-09 14 10 38

Prefetch storage waste is also same. It means the scenario fixed by this pull request (storage is modified and then set back to its original value within the same transaction) doesn't happen a lot.

截屏2024-05-09 14 13 02

@rjl493456442 rjl493456442 modified the milestones: 1.14.3, 1.14.4 May 9, 2024
@karalabe karalabe merged commit e5f5eae into ethereum:master May 10, 2024
3 checks passed
jorgemmsilva pushed a commit to iotaledger/go-ethereum that referenced this pull request Jun 17, 2024
…thereum#29731)

* core/state: remove slot dirtiness if it's set back to origin value

* core/state: suggestion from martin
stwiname pushed a commit to subquery/data-node-go-ethereum that referenced this pull request Sep 9, 2024
…thereum#29731)

* core/state: remove slot dirtiness if it's set back to origin value

* core/state: suggestion from martin
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