-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[R4R]add sharedStorage for prefetching to L1 #792
Conversation
2840a8c
to
8bccd6f
Compare
8ecd085
to
3338f74
Compare
3338f74
to
7abd888
Compare
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.
LGTM
core/state/shared_pool.go
Outdated
// sharedStorage is used to store maps of originStorage of stateObjects | ||
type SharedStorage struct { | ||
poolLock *sync.RWMutex | ||
shared_map map[common.Address]*sync.Map |
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.
"shared_map" needs to keep the same naming style with poolLock
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.
updated
core/blockchain.go
Outdated
go func(start time.Time, followup *types.Block, throwaway *state.StateDB, interrupt *uint32) { | ||
bc.prefetcher.Prefetch(followup, throwaway, bc.vmConfig, &followupInterrupt) | ||
}(time.Now(), block, throwaway, &followupInterrupt) | ||
}(time.Now(), block, statedb, &followupInterrupt) |
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.
Would not suggest to share StateDB between Prefetcher and main StateDB.
Use prefetch DB instead, main StateDB can access prefetch DB to get the originStorage
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.
Prefetch DB could be used as: L1.5 Cache
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.
The stateDB of main process an prefetcher are different, they only share the sharedpool which store the originStroage maps , I had adjust the sharedPool to be L1.5 Cache by the new commit
core/blockchain.go
Outdated
go func(start time.Time, followup *types.Block, throwaway *state.StateDB, interrupt *uint32) { | ||
bc.prefetcher.Prefetch(followup, throwaway, bc.vmConfig, &followupInterrupt) | ||
}(time.Now(), block, throwaway, &followupInterrupt) | ||
}(time.Now(), block, statedb, &followupInterrupt) |
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.
Would not suggest to share StateDB between Prefetcher and main StateDB.
Use prefetch DB instead, main StateDB can access prefetch DB to get the originStorage
core/state/shared_pool.go
Outdated
storage.RUnlock() | ||
if !ok { | ||
m := new(sync.Map) | ||
storage.Lock() |
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.
concurrent race condition?
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.
yes, newObjects called by main preoces or prefetcher will both call this function to check or update the sharedpool
f886006
to
e031e4e
Compare
5c2222a
to
85bd395
Compare
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.
Please fix lint.
core/state/shared_pool.go
Outdated
storageMap, ok := s.sharedMap[address] | ||
s.RUnlock() | ||
if !ok { | ||
m := new(sync.Map) |
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.
Has race condition.
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.
updated
a25f788
to
e2e899f
Compare
e2e899f
to
76dac12
Compare
core/state/shared_pool.go
Outdated
} | ||
} | ||
|
||
// Check whether the storage exist in pool, |
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.
Would be better to follow the commenting style used in go-ethereum, like, // Copy creates a deep, independent copy of the state. // Snapshots of the copied state cannot be applied to the copy. func (s *StateDB) Copy() *StateDB {
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.
updated
LGTM |
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.
LGTM
@@ -204,7 +236,8 @@ func (s *StateObject) GetCommittedState(db Database, key common.Hash) common.Has | |||
if value, pending := s.pendingStorage[key]; pending { | |||
return value | |||
} | |||
if value, cached := s.originStorage[key]; cached { | |||
|
|||
if value, cached := s.getOriginStorage(key); cached { |
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.
Can leave it unchanged; the API: getOriginStorage
can be removed;
// Try to get it from origin storage prefetch pool
if s.sharedOriginStorage != nil {
val, ok := s.sharedOriginStorage.Load(key)
if ok {
s.originStorage[key] = val.(common.Hash)
return val.(common.Hash), true
}
}
return common.Hash{}, false | ||
} | ||
|
||
func (s *StateObject) setOriginStorage(key common.Hash, value common.Hash) { |
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.
No need to add a new APIsetOriginStorage
either
@@ -0,0 +1,37 @@ | |||
package state |
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.
How about rename the file with: shared_storage_pool.go
, shared_pool.go
is a bit confusing, shared what?
@@ -1633,3 +1575,7 @@ func (s *StateDB) GetDirtyAccounts() []common.Address { | |||
} | |||
return accounts | |||
} | |||
|
|||
func (s *StateDB) GetStorage(address common.Address) *sync.Map { |
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.
No need to addGetStorage
;
GetStorage
is paired with SetStorage
, it is for fakeStorage.
Do not occupy the name.
@@ -79,7 +80,9 @@ type StateObject struct { | |||
trie Trie // storage trie, which becomes non-nil on first access | |||
code Code // contract bytecode, which gets set when code is loaded | |||
|
|||
originStorage Storage // Storage cache of original entries to dedup rewrites, reset for every transaction | |||
sharedOriginStorage *sync.Map // Storage cache of original entries to dedup rewrites, reset for every transaction |
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.
// Storage cache of original entries to dedup rewrites, reset for every transaction
the comment is for originStorage, pls add a new comment for sharedOriginStorage
@@ -120,14 +123,21 @@ func newObject(db *StateDB, address common.Address, data Account) *StateObject { | |||
if data.Root == (common.Hash{}) { | |||
data.Root = emptyRoot | |||
} | |||
var storageMap *sync.Map |
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.
rename storageMap
to sharedStorage
, storage is a Map by itself.
@@ -101,6 +98,8 @@ type StateDB struct { | |||
stateObjectsPending map[common.Address]struct{} // State objects finalized but not yet written to the trie | |||
stateObjectsDirty map[common.Address]struct{} // State objects modified in the current execution | |||
|
|||
storagePool *StoragePool // sharedPool to store L1 originStorage of stateObjects |
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.
name: storagePool
-> originStoragePool
to make it clear.
@@ -101,6 +98,8 @@ type StateDB struct { | |||
stateObjectsPending map[common.Address]struct{} // State objects finalized but not yet written to the trie | |||
stateObjectsDirty map[common.Address]struct{} // State objects modified in the current execution | |||
|
|||
storagePool *StoragePool // sharedPool to store L1 originStorage of stateObjects | |||
writeOnSharedStorage bool // Write to the shared origin storage of a stateObject while reading from the underlying storage layer. |
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.
rename writeOnSharedStorage
to updateSharedOriginStorage
* add sharedStorage for prefetching to L1 * remote originStorage in stateObjects * fix core * fix bug of sync map * remove read lock when get & set keys * statedb copy use CopyWithSharedStorage * reduce lock access * fix comment * avoid sharedPool effects on other modules * remove tryPreload * fix comment * fix var name * fix lint * fix L1 miss data && data condition * fix comment
* add sharedStorage for prefetching to L1 * remote originStorage in stateObjects * fix core * fix bug of sync map * remove read lock when get & set keys * statedb copy use CopyWithSharedStorage * reduce lock access * fix comment * avoid sharedPool effects on other modules * remove tryPreload * fix comment * fix var name * fix lint * fix L1 miss data && data condition * fix comment
Description: This PR refers to the code optimization done by `BSC`, which mainly includes the following parts: 1. Do BlockBody verification concurrently. 2. Do the calculation of intermediate root concurrently. 3. Commit the MPTs concurrently. 4. Preload accounts before processing blocks. 5. Make the snapshot layers configurable. 6. Reuse some objects to reduce GC. 7. Add shared_pool for `stateDB` to improve cache usage. References: - bnb-chain/bsc#257 - bnb-chain/bsc#792 --------- Co-authored-by: j75689 <j75689@gmail.com> Co-authored-by: realowen <131384228+realowen@users.noreply.github.com>
Description This PR refers to the code optimization done by `BSC`, which mainly includes the following parts: 1. Do BlockBody verification concurrently. 2. Do the calculation of intermediate root concurrently. 3. Commit the MPTs concurrently. 4. Preload accounts before processing blocks. 5. Make the snapshot layers configurable. 6. Reuse some objects to reduce GC. 7. Add shared_pool for `stateDB` to improve cache usage. References - bnb-chain/bsc#257 - bnb-chain/bsc#792 --------- Co-authored-by: j75689 <j75689@gmail.com> Co-authored-by: realowen <131384228+realowen@users.noreply.github.com>
Description This PR refers to the code optimization done by `BSC`, which mainly includes the following parts: 1. Do BlockBody verification concurrently. 2. Do the calculation of intermediate root concurrently. 3. Commit the MPTs concurrently. 4. Preload accounts before processing blocks. 5. Make the snapshot layers configurable. 6. Reuse some objects to reduce GC. 7. Add shared_pool for `stateDB` to improve cache usage. References - bnb-chain/bsc#257 - bnb-chain/bsc#792 --------- Co-authored-by: j75689 <j75689@gmail.com>
Description
In the previous prefetch behavior of importing blocks, the prefetch and the main process use completely different state dbs, the prefetch process can only put the retrieved data into the L3 layer, if the main process wants to access, it must go through the L1-L2-L3 layers, the new design let the main process and the prefetch process share a shared_pool with the stateDB. This pool stores the originStorage data (each stateobjects has the corresponding
entry ). By this way , the prefetch data that can be directly fished to the L1 layer
This shared_pool can be represented as the following secondary map structure, which is a private member variable in stateDB , each stateObjects has pointer to access the Corresponding entry of shared_pool
This PR also remove the overhead of TryPreload which cost 2%-3% in main process.
Rationale
we can make a lot of data in the main process that originally need to be accessed in L3 directly accessed in L1 layer, which reduces a lot of L2 and L3 layer overhead in the number of layers
Performance
The two test machine is on AWS , the two nodes has worked in fullsync mode and has caught up to the latest block
This is the delay of process block , we can see the machine which running master branch data has nearly same performance data
Then update a machine with this PR code, it can be clearly seen that the delay data is reduced (green performance data), and the average process delay is reduced by about 7%
The proportion of I/O overhead in the total block cost of the main process dropped from 33% to 30.5%
The overhead of total block cost has reduce from to 3.67% to 1.1% with PR code
Example
add an example CLI or API response...
Changes
Notable changes: