-
Notifications
You must be signed in to change notification settings - Fork 759
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
statemanager: cache and other refactors #3569
Merged
Merged
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
fa3c2ec
Revert "statemanager: refactor logic into capabilities (#3554)"
gabrocheleau bd1f237
statemanager: refactor modifyAccountFields
gabrocheleau d5aaf30
statemanager: adjust return statements
gabrocheleau 533fa8c
statemanager: refactor separate caches into caches class
gabrocheleau 31c94af
chore: merge with master
gabrocheleau bba928b
statemanager: fix shallow copy logic
gabrocheleau a04158d
client: fix vmexecution statemanager cache opts
gabrocheleau 2431594
statemanager: refactor some capabilities to caches methods
gabrocheleau 814b9f7
statemanager: fix tests
gabrocheleau 2439219
statemanager: refactor caches into optional opt passed in directly to…
gabrocheleau 4c6240f
statemanager: adjust tests with refactored caches
gabrocheleau 5e1666d
client: adjust vm execution with update cache
gabrocheleau c754eac
vm: fix vm tests
gabrocheleau 5d4895c
vm: adjust test runners with non-default caches
gabrocheleau 888b20e
statemanager: simplify handling of deactivate
gabrocheleau 905a52e
statemanager: remove redundant checks
gabrocheleau f30288a
statemanager: remove non null asesertion
gabrocheleau ba00837
statemanager: remove cache opt from key naming
gabrocheleau c037ae2
statemanager: refactor rpc state manager to use caches
gabrocheleau fdcdb36
statemanager: fix rpc state manager tests
gabrocheleau 3d93587
client: vmexecution cache stats refactor
gabrocheleau 48baee3
Merge remote-tracking branch 'origin/master' into statemanager/capabi…
acolytec3 f0b66bf
Merge branch 'master' into statemanager/capabilities-refactor
acolytec3 5f4c253
statemanageR: updategetproof json-rpc call format
gabrocheleau 7fe21fd
statemanager: remove deactivate from caches
gabrocheleau 4864383
client: remove deactivate from vm execution instantiation
gabrocheleau b6ee97b
Merge branch 'statemanager/capabilities-refactor' of https://github.c…
gabrocheleau File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
import { AccountCache } from './account.js' | ||
import { CodeCache } from './code.js' | ||
import { StorageCache } from './storage.js' | ||
import { type CacheSettings, CacheType, type CachesStateManagerOpts } from './types.js' | ||
|
||
import type { Address } from '@ethereumjs/util' | ||
|
||
export class Caches { | ||
account?: AccountCache | ||
code?: CodeCache | ||
storage?: StorageCache | ||
|
||
settings: Record<'account' | 'code' | 'storage', CacheSettings> | ||
|
||
constructor(opts: CachesStateManagerOpts = {}) { | ||
const accountSettings = { | ||
deactivate: (opts.account?.deactivate === true || opts.account?.size === 0) ?? false, | ||
type: opts.account?.type ?? CacheType.ORDERED_MAP, | ||
size: opts.account?.size ?? 100000, | ||
} | ||
const storageSettings = { | ||
deactivate: (opts.storage?.deactivate === true || opts.storage?.size === 0) ?? false, | ||
type: opts.storage?.type ?? CacheType.ORDERED_MAP, | ||
size: opts.storage?.size ?? 20000, | ||
} | ||
|
||
const codeSettings = { | ||
deactivate: (opts.code?.deactivate === true || opts.code?.size === 0) ?? false, | ||
type: opts.code?.type ?? CacheType.ORDERED_MAP, | ||
size: opts.code?.size ?? 20000, | ||
} | ||
|
||
this.settings = { | ||
account: accountSettings, | ||
code: codeSettings, | ||
storage: storageSettings, | ||
} | ||
|
||
if (this.settings.account.deactivate === false) { | ||
this.account = new AccountCache({ | ||
size: this.settings.account.size, | ||
type: this.settings.account.type, | ||
}) | ||
} | ||
|
||
if (this.settings.storage.deactivate === false) { | ||
this.storage = new StorageCache({ | ||
size: this.settings.storage.size, | ||
type: this.settings.storage.type, | ||
}) | ||
} | ||
|
||
if (this.settings.code.deactivate === false) { | ||
this.code = new CodeCache({ | ||
size: this.settings.code.size, | ||
type: this.settings.code.type, | ||
}) | ||
} | ||
} | ||
|
||
checkpoint() { | ||
this.account?.checkpoint() | ||
this.storage?.checkpoint() | ||
this.code?.checkpoint() | ||
} | ||
|
||
clear() { | ||
this.account?.clear() | ||
this.storage?.clear() | ||
this.code?.clear() | ||
} | ||
|
||
commit() { | ||
this.account?.commit() | ||
this.storage?.commit() | ||
this.code?.commit() | ||
} | ||
|
||
deleteAccount(address: Address) { | ||
this.code?.del(address) | ||
this.account?.del(address) | ||
this.storage?.clearStorage(address) | ||
} | ||
|
||
revert() { | ||
this.account?.revert() | ||
this.storage?.revert() | ||
this.code?.revert() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,58 @@ | ||
import type { CacheType } from '@ethereumjs/common' | ||
export enum CacheType { | ||
LRU = 'lru', | ||
ORDERED_MAP = 'ordered_map', | ||
} | ||
|
||
export interface CacheOpts { | ||
size: number | ||
type: CacheType | ||
} | ||
|
||
export type CacheSettings = { | ||
deactivate: boolean | ||
type: CacheType | ||
size: number | ||
} | ||
|
||
export interface CacheStateManagerOpts { | ||
/** | ||
* Allows for cache deactivation | ||
* | ||
* Depending on the use case and underlying datastore (and eventual concurrent cache | ||
* mechanisms there), usage with or without cache can be faster | ||
* | ||
* Default: false | ||
*/ | ||
deactivate?: boolean | ||
|
||
/** | ||
* Cache type to use. | ||
* | ||
* Available options: | ||
* | ||
* ORDERED_MAP: Cache with no fixed upper bound and dynamic allocation, | ||
* use for dynamic setups like testing or similar. | ||
* | ||
* LRU: LRU cache with pre-allocation of memory and a fixed size. | ||
* Use for larger and more persistent caches. | ||
*/ | ||
type?: CacheType | ||
|
||
/** | ||
* Size of the cache (only for LRU cache) | ||
* | ||
* Default: 100000 (account cache) / 20000 (storage cache) / 20000 (code cache) | ||
* | ||
* Note: the cache/trie interplay mechanism is designed in a way that | ||
* the theoretical number of max modified accounts between two flush operations | ||
* should be smaller than the cache size, otherwise the cache will "forget" the | ||
* old modifications resulting in an incomplete set of trie-flushed accounts. | ||
*/ | ||
size?: number | ||
} | ||
|
||
export interface CachesStateManagerOpts { | ||
account?: CacheStateManagerOpts | ||
code?: CacheStateManagerOpts | ||
storage?: CacheStateManagerOpts | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe drop the
StateManager
here from the nameThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we remove StateManager, we'll have a naming conflict with
CacheOpts
declared above. I am not 100% satisfied with the general naming, and there is some overlap between these interfaces/types (which makes me want to unify them somehow), but there does not seem to be sufficient overlap to really unify them. Open to suggestions!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe give this a second thought, but I would kill off the
deactivate
part and take thesize === 0
as the one and only way to deactivate and then remove theCacheOpts
and only use theCachesStateManagerOpts
(then renamed toCacheOpts
😋) as the one thing for all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, just seeing this comment I'll give that a try, would really like to simplify further