From b50f8660cc0fb8f1ea75c08322cac79c824c106c Mon Sep 17 00:00:00 2001 From: ahamlat Date: Wed, 2 Nov 2022 18:05:15 +0100 Subject: [PATCH] Create a specific calculateRootHash method for BonsaiInMemoryWorldState to improve performance (#4568) * Parallelize some steps in BonsaiPersistedWorldState.calculateRootHash method Signed-off-by: Ameziane H * Add synchronized on storage flat database remove and update method Signed-off-by: Ameziane H * Add synchronized on storage flat database remove and update method Signed-off-by: Ameziane H * Fix this error org.rocksdb.RocksDBException: unknown WriteBatch tag Signed-off-by: Ameziane H * create a specific calculateRootHash for BonsaiInMemoryWorldState class Signed-off-by: Ameziane H * create a specific calculateRootHash for BonsaiInMemoryWorldState class Signed-off-by: Ameziane H * Fix nullPointerException on Collections.synchronizedSet Signed-off-by: Ameziane H * Use parallelStreams instead of CompletableFuture API Signed-off-by: Ameziane H * Modify CHANGELOG.md Signed-off-by: Ameziane H * spotless and synchronizedSet initializaton Signed-off-by: garyschulte Signed-off-by: Ameziane H Signed-off-by: garyschulte Co-authored-by: garyschulte Co-authored-by: Sally MacFarlane --- CHANGELOG.md | 1 + .../bonsai/BonsaiInMemoryWorldState.java | 98 +++++++++- .../bonsai/BonsaiPersistedWorldState.java | 181 ++++++++++-------- .../BonsaiWorldStateKeyValueStorage.java | 5 +- .../bonsai/BonsaiWorldStateUpdater.java | 8 +- .../evm/worldstate/AbstractWorldUpdater.java | 7 +- .../RocksDBColumnarKeyValueStorage.java | 2 +- 7 files changed, 208 insertions(+), 94 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b12eb009b30..be6521e960c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ - Upgrade RocksDB database version from 6.29.5 to 7.6.0 [#4517](https://github.com/hyperledger/besu/pull/4517) - Avoid connecting to self when using static-nodes [#4521](https://github.com/hyperledger/besu/pull/4521) - EVM performance has increased 20%-100% depending on the particulars of the contract. [#4540](https://github.com/hyperledger/besu/pull/4540) +- Improve calculateRootHash method performance during Block processing [#4568](https://github.com/hyperledger/besu/pull/4568) ### Bug Fixes - Corrects emission of blockadded events when rewinding during a re-org. Fix for [#4495](https://github.com/hyperledger/besu/issues/4495) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java index 7f29433e26b..8d04717ff4e 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java @@ -16,8 +16,16 @@ package org.hyperledger.besu.ethereum.bonsai; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.trie.StoredMerklePatriciaTrie; + +import java.util.Map; +import java.util.function.Function; + +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.units.bigints.UInt256; public class BonsaiInMemoryWorldState extends BonsaiPersistedWorldState { @@ -38,12 +46,90 @@ public Hash rootHash() { } public Hash rootHash(final BonsaiWorldStateUpdater localUpdater) { - final BonsaiWorldStateKeyValueStorage.BonsaiUpdater updater = worldStateStorage.updater(); - try { - final Hash calculatedRootHash = calculateRootHash(updater, localUpdater); - return Hash.wrap(calculatedRootHash); - } finally { - updater.rollback(); + final Hash calculatedRootHash = calculateRootHash(localUpdater); + return Hash.wrap(calculatedRootHash); + } + + protected Hash calculateRootHash(final BonsaiWorldStateUpdater worldStateUpdater) { + + // second update account storage state. This must be done before updating the accounts so + // that we can get the storage state hash + + worldStateUpdater.getStorageToUpdate().entrySet().parallelStream() + .forEach( + addressMapEntry -> { + updateAccountStorage(worldStateUpdater, addressMapEntry); + }); + + // for manicured tries and composting, trim and compost here + + // next walk the account trie + final StoredMerklePatriciaTrie accountTrie = + new StoredMerklePatriciaTrie<>( + this::getAccountStateTrieNode, + worldStateRootHash, + Function.identity(), + Function.identity()); + + // for manicured tries and composting, collect branches here (not implemented) + + // now add the accounts + for (final Map.Entry> accountUpdate : + worldStateUpdater.getAccountsToUpdate().entrySet()) { + final Bytes accountKey = accountUpdate.getKey(); + final BonsaiValue bonsaiValue = accountUpdate.getValue(); + final BonsaiAccount updatedAccount = bonsaiValue.getUpdated(); + if (updatedAccount == null) { + final Hash addressHash = Hash.hash(accountKey); + accountTrie.remove(addressHash); + } else { + final Hash addressHash = updatedAccount.getAddressHash(); + final Bytes accountValue = updatedAccount.serializeAccount(); + accountTrie.put(addressHash, accountValue); + } + } + + // TODO write to a cache and then generate a layer update from that and the + // DB tx updates. Right now it is just DB updates. + return Hash.wrap(accountTrie.getRootHash()); + } + + private void updateAccountStorage( + final BonsaiWorldStateUpdater worldStateUpdater, + final Map.Entry>> storageAccountUpdate) { + final Address updatedAddress = storageAccountUpdate.getKey(); + final Hash updatedAddressHash = Hash.hash(updatedAddress); + if (worldStateUpdater.getAccountsToUpdate().containsKey(updatedAddress)) { + final BonsaiValue accountValue = + worldStateUpdater.getAccountsToUpdate().get(updatedAddress); + final BonsaiAccount accountOriginal = accountValue.getPrior(); + final Hash storageRoot = + (accountOriginal == null) ? Hash.EMPTY_TRIE_HASH : accountOriginal.getStorageRoot(); + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + (location, key) -> getStorageTrieNode(updatedAddressHash, location, key), + storageRoot, + Function.identity(), + Function.identity()); + + // for manicured tries and composting, collect branches here (not implemented) + + for (final Map.Entry> storageUpdate : + storageAccountUpdate.getValue().entrySet()) { + final Hash keyHash = storageUpdate.getKey(); + final UInt256 updatedStorage = storageUpdate.getValue().getUpdated(); + if (updatedStorage == null || updatedStorage.equals(UInt256.ZERO)) { + storageTrie.remove(keyHash); + } else { + storageTrie.put(keyHash, BonsaiWorldView.encodeTrieValue(updatedStorage)); + } + } + + final BonsaiAccount accountUpdated = accountValue.getUpdated(); + if (accountUpdated != null) { + final Hash newStorageRoot = Hash.wrap(storageTrie.getRootHash()); + accountUpdated.setStorageRoot(newStorageRoot); + } } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java index 7c7e70257ce..d2383c8b461 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java @@ -104,43 +104,76 @@ public BonsaiWorldStateKeyValueStorage getWorldStateStorage() { protected Hash calculateRootHash( final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater, final BonsaiWorldStateUpdater worldStateUpdater) { - // first clear storage - for (final Address address : worldStateUpdater.getStorageToClear()) { - // because we are clearing persisted values we need the account root as persisted - final BonsaiAccount oldAccount = - worldStateStorage - .getAccount(Hash.hash(address)) - .map(bytes -> fromRLP(BonsaiPersistedWorldState.this, address, bytes, true)) - .orElse(null); - if (oldAccount == null) { - // This is when an account is both created and deleted within the scope of the same - // block. A not-uncommon DeFi bot pattern. - continue; + clearStorage(stateUpdater, worldStateUpdater); + + // This must be done before updating the accounts so + // that we can get the storage state hash + updateAccountStorageState(stateUpdater, worldStateUpdater); + + // Third update the code. This has the side effect of ensuring a code hash is calculated. + updateCode(stateUpdater, worldStateUpdater); + + // next walk the account trie + final StoredMerklePatriciaTrie accountTrie = + new StoredMerklePatriciaTrie<>( + this::getAccountStateTrieNode, + worldStateRootHash, + Function.identity(), + Function.identity()); + + // for manicured tries and composting, collect branches here (not implemented) + + addTheAccounts(stateUpdater, worldStateUpdater, accountTrie); + + // TODO write to a cache and then generate a layer update from that and the + // DB tx updates. Right now it is just DB updates. + accountTrie.commit( + (location, hash, value) -> + writeTrieNode(stateUpdater.getTrieBranchStorageTransaction(), location, value)); + final Bytes32 rootHash = accountTrie.getRootHash(); + return Hash.wrap(rootHash); + } + + private static void addTheAccounts( + final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater, + final BonsaiWorldStateUpdater worldStateUpdater, + final StoredMerklePatriciaTrie accountTrie) { + for (final Map.Entry> accountUpdate : + worldStateUpdater.getAccountsToUpdate().entrySet()) { + final Bytes accountKey = accountUpdate.getKey(); + final BonsaiValue bonsaiValue = accountUpdate.getValue(); + final BonsaiAccount updatedAccount = bonsaiValue.getUpdated(); + if (updatedAccount == null) { + final Hash addressHash = Hash.hash(accountKey); + accountTrie.remove(addressHash); + stateUpdater.removeAccountInfoState(addressHash); + } else { + final Hash addressHash = updatedAccount.getAddressHash(); + final Bytes accountValue = updatedAccount.serializeAccount(); + stateUpdater.putAccountInfoState(Hash.hash(accountKey), accountValue); + accountTrie.put(addressHash, accountValue); } - final Hash addressHash = Hash.hash(address); - final StoredMerklePatriciaTrie storageTrie = - new StoredMerklePatriciaTrie<>( - (location, key) -> getStorageTrieNode(addressHash, location, key), - oldAccount.getStorageRoot(), - Function.identity(), - Function.identity()); - Map entriesToDelete = storageTrie.entriesFrom(Bytes32.ZERO, 256); - while (!entriesToDelete.isEmpty()) { - entriesToDelete - .keySet() - .forEach( - k -> stateUpdater.removeStorageValueBySlotHash(Hash.hash(address), Hash.wrap(k))); - if (entriesToDelete.size() == 256) { - entriesToDelete.keySet().forEach(storageTrie::remove); - entriesToDelete = storageTrie.entriesFrom(Bytes32.ZERO, 256); - } else { - break; - } + } + } + + private static void updateCode( + final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater, + final BonsaiWorldStateUpdater worldStateUpdater) { + for (final Map.Entry> codeUpdate : + worldStateUpdater.getCodeToUpdate().entrySet()) { + final Bytes updatedCode = codeUpdate.getValue().getUpdated(); + final Hash accountHash = Hash.hash(codeUpdate.getKey()); + if (updatedCode == null || updatedCode.size() == 0) { + stateUpdater.removeCode(accountHash); + } else { + stateUpdater.putCode(accountHash, null, updatedCode); } } + } - // second update account storage state. This must be done before updating the accounts so - // that we can get the storage state hash + private void updateAccountStorageState( + final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater, + final BonsaiWorldStateUpdater worldStateUpdater) { for (final Map.Entry>> storageAccountUpdate : worldStateUpdater.getStorageToUpdate().entrySet()) { final Address updatedAddress = storageAccountUpdate.getKey(); @@ -184,54 +217,44 @@ protected Hash calculateRootHash( } // for manicured tries and composting, trim and compost here } + } - // Third update the code. This has the side effect of ensuring a code hash is calculated. - for (final Map.Entry> codeUpdate : - worldStateUpdater.getCodeToUpdate().entrySet()) { - final Bytes updatedCode = codeUpdate.getValue().getUpdated(); - final Hash accountHash = Hash.hash(codeUpdate.getKey()); - if (updatedCode == null || updatedCode.size() == 0) { - stateUpdater.removeCode(accountHash); - } else { - stateUpdater.putCode(accountHash, null, updatedCode); + private void clearStorage( + final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater, + final BonsaiWorldStateUpdater worldStateUpdater) { + for (final Address address : worldStateUpdater.getStorageToClear()) { + // because we are clearing persisted values we need the account root as persisted + final BonsaiAccount oldAccount = + worldStateStorage + .getAccount(Hash.hash(address)) + .map(bytes -> fromRLP(BonsaiPersistedWorldState.this, address, bytes, true)) + .orElse(null); + if (oldAccount == null) { + // This is when an account is both created and deleted within the scope of the same + // block. A not-uncommon DeFi bot pattern. + continue; } - } - - // next walk the account trie - final StoredMerklePatriciaTrie accountTrie = - new StoredMerklePatriciaTrie<>( - this::getAccountStateTrieNode, - worldStateRootHash, - Function.identity(), - Function.identity()); - - // for manicured tries and composting, collect branches here (not implemented) - - // now add the accounts - for (final Map.Entry> accountUpdate : - worldStateUpdater.getAccountsToUpdate().entrySet()) { - final Bytes accountKey = accountUpdate.getKey(); - final BonsaiValue bonsaiValue = accountUpdate.getValue(); - final BonsaiAccount updatedAccount = bonsaiValue.getUpdated(); - if (updatedAccount == null) { - final Hash addressHash = Hash.hash(accountKey); - accountTrie.remove(addressHash); - stateUpdater.removeAccountInfoState(addressHash); - } else { - final Hash addressHash = updatedAccount.getAddressHash(); - final Bytes accountValue = updatedAccount.serializeAccount(); - stateUpdater.putAccountInfoState(Hash.hash(accountKey), accountValue); - accountTrie.put(addressHash, accountValue); + final Hash addressHash = Hash.hash(address); + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + (location, key) -> getStorageTrieNode(addressHash, location, key), + oldAccount.getStorageRoot(), + Function.identity(), + Function.identity()); + Map entriesToDelete = storageTrie.entriesFrom(Bytes32.ZERO, 256); + while (!entriesToDelete.isEmpty()) { + entriesToDelete + .keySet() + .forEach( + k -> stateUpdater.removeStorageValueBySlotHash(Hash.hash(address), Hash.wrap(k))); + entriesToDelete.keySet().forEach(storageTrie::remove); + if (entriesToDelete.size() == 256) { + entriesToDelete = storageTrie.entriesFrom(Bytes32.ZERO, 256); + } else { + break; + } } } - - // TODO write to a cache and then generate a layer update from that and the - // DB tx updates. Right now it is just DB updates. - accountTrie.commit( - (location, hash, value) -> - writeTrieNode(stateUpdater.getTrieBranchStorageTransaction(), location, value)); - final Bytes32 rootHash = accountTrie.getRootHash(); - return Hash.wrap(rootHash); } @Override @@ -342,7 +365,7 @@ public Account get(final Address address) { .orElse(null); } - private Optional getAccountStateTrieNode(final Bytes location, final Bytes32 nodeHash) { + protected Optional getAccountStateTrieNode(final Bytes location, final Bytes32 nodeHash) { return worldStateStorage.getAccountStateTrieNode(location, nodeHash); } @@ -351,7 +374,7 @@ private void writeTrieNode( tx.put(location.toArrayUnsafe(), value.toArrayUnsafe()); } - private Optional getStorageTrieNode( + protected Optional getStorageTrieNode( final Hash accountHash, final Bytes location, final Bytes32 nodeHash) { return worldStateStorage.getAccountStorageTrieNode(accountHash, location, nodeHash); } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java index cce53b7e29f..047593e082a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java @@ -378,7 +378,7 @@ public BonsaiUpdater putAccountStorageTrieNode( } @Override - public BonsaiUpdater putStorageValueBySlotHash( + public synchronized BonsaiUpdater putStorageValueBySlotHash( final Hash accountHash, final Hash slotHash, final Bytes storage) { storageStorageTransaction.put( Bytes.concatenate(accountHash, slotHash).toArrayUnsafe(), storage.toArrayUnsafe()); @@ -386,7 +386,8 @@ public BonsaiUpdater putStorageValueBySlotHash( } @Override - public void removeStorageValueBySlotHash(final Hash accountHash, final Hash slotHash) { + public synchronized void removeStorageValueBySlotHash( + final Hash accountHash, final Hash slotHash) { storageStorageTransaction.remove(Bytes.concatenate(accountHash, slotHash).toArrayUnsafe()); } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java index cafe6c93bf9..e6d00b8063d 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java @@ -28,6 +28,7 @@ import org.hyperledger.besu.evm.worldstate.WrappedEvmAccount; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -47,9 +48,10 @@ public class BonsaiWorldStateUpdater extends AbstractWorldUpdater implements BonsaiWorldView { - private final Map> accountsToUpdate = new HashMap<>(); - private final Map> codeToUpdate = new HashMap<>(); - private final Set
storageToClear = new HashSet<>(); + private final Map> accountsToUpdate = + new ConcurrentHashMap<>(); + private final Map> codeToUpdate = new ConcurrentHashMap<>(); + private final Set
storageToClear = Collections.synchronizedSet(new HashSet<>()); // storage sub mapped by _hashed_ key. This is because in self_destruct calls we need to // enumerate the old storage and delete it. Those are trie stored by hashed key by spec and the diff --git a/evm/src/main/java/org/hyperledger/besu/evm/worldstate/AbstractWorldUpdater.java b/evm/src/main/java/org/hyperledger/besu/evm/worldstate/AbstractWorldUpdater.java index ea2ca2b9afa..a8f2f708a4e 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/worldstate/AbstractWorldUpdater.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/worldstate/AbstractWorldUpdater.java @@ -21,11 +21,12 @@ import org.hyperledger.besu.evm.account.MutableAccount; import java.util.Collection; -import java.util.HashMap; +import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** * An abstract implementation of a {@link WorldUpdater} that buffers update over the {@link @@ -38,8 +39,8 @@ public abstract class AbstractWorldUpdater> updatedAccounts = new HashMap<>(); - protected Set
deletedAccounts = new HashSet<>(); + protected Map> updatedAccounts = new ConcurrentHashMap<>(); + protected Set
deletedAccounts = Collections.synchronizedSet(new HashSet<>()); protected AbstractWorldUpdater(final W world) { this.world = world; diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java index 2333e53c1de..564a9be9eb2 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java @@ -336,7 +336,7 @@ public void remove(final RocksDbSegmentIdentifier segment, final byte[] key) { } @Override - public void commit() throws StorageException { + public synchronized void commit() throws StorageException { try (final OperationTimer.TimingContext ignored = metrics.getCommitLatency().startTimer()) { innerTx.commit(); } catch (final RocksDBException e) {