perf: optimize `debug_storageRangeAt` #809

davidmurdoch · 2021-02-26T02:24:42Z

No description provided.

davidmurdoch · 2021-02-26T02:25:11Z

src/chains/ethereum/ethereum/src/@types/merkle-patricia-tree/index.d.ts

    get(key: LargeNumber, cb: Callback<Buffer | null>): void;
    put(key: LargeNumber, value: LargeNumber, cb: Callback<never>): void;
-    copy(): Trie;
+    copy(): CheckpointTrie;


Just fixing some types

davidmurdoch · 2021-02-26T02:25:24Z

src/chains/ethereum/ethereum/src/@types/merkle-patricia-tree/secure.d.ts

@@ -0,0 +1,11 @@
+declare module "merkle-patricia-tree/secure" {


Just adding a type for SecureTrie

davidmurdoch · 2021-02-26T02:29:21Z

src/chains/ethereum/ethereum/src/blockchain.ts

-      if (txHashCurrentlyProcessing == transactionHash) {
+    const beforeTxListener = (tx: Transaction) => {
+      txHashCurrentlyProcessing = tx.hash();
+      if (txHashCurrentlyProcessing.equals(transactionHash)) {


Comparing buffers is faster than converting them to a string then comparing strings.

davidmurdoch · 2021-02-26T02:30:04Z

src/chains/ethereum/ethereum/src/blockchain.ts

-              (err: Error, result: Buffer) => {
-                if (err) {
-                  throw err;
+          const database = this.#database;


For reasons, this.#database is slow once typescript compiles things, so I like to cache the lookup.

How did you measure the performance difference @davidmurdoch ?

Don't need to. Just look at what typescript generates for our ES target. It's a function call, a Map has call, a branch, and a Map get call, IIRC.

I'd love to generate multiple builds so we can ship without these sorts of polyfills. I like the "true" private fields the syntax provides, and don't think I want to remove their use all together.

Happy to have your review on this!

davidmurdoch · 2021-02-26T02:31:31Z

src/chains/ethereum/ethereum/src/blockchain.ts

-                if (err) {
-                  throw err;
+          const database = this.#database;
+          return Promise.all(


The beforeTx event emitter awaits the returned promise. I didn't find any race condition issues in our tests, but this seemed like a place they could crop up, so I thought it'd be good to "fix" it anyway.

davidmurdoch · 2021-02-26T02:43:40Z

src/chains/ethereum/ethereum/src/blockchain.ts


-            resolve(keys);
-          });
+        const rs = storageTrie.createReadStream();


We can't get keys to come back already ordered because the merkle patricia tree library we use parallelizes up to 500 database requests at once.

This just means we have to sort client side, which could mean performance takes a hit.

But generally I think callers to debug_storageRangeAt will attempt to consume all available storage in one go, which means we would have had to load all the data anyway.

Where we'd take a hit is when we only need to return a small amount of storage. In that case we still have to load all of it.

I think turning the priority queue it uses internally into a FIFO queue would actually be very complicated, because it resolves both branches and leaves, and it'd have to ensure the proper ordering there. Not positive though.

Since that's not going to happen any time soon, the next best thing is to just insert them in order as we get them, instead of a big oll sort at the end.

This will suck a bit more after we get the in-memory cache in place. We won't need any parallelization in that case; and it'll actually just slow everything down (it'll still be faster than hitting leveldb, of course).

Thanks for watching. Don't forget to smash that like button!

src/chains/ethereum/ethereum/src/blockchain.ts

davidmurdoch · 2021-02-26T02:45:47Z

src/chains/ethereum/ethereum/src/database.ts

  #initialize = async () => {
-    const levelupOptions: any = { valueEncoding: "binary" };
+    const levelupOptions: any = {
+      keyEncoding: "binary",


This makes it so that we don't have to stringify our keys, which are almost always, if not always, already a Buffer.

davidmurdoch · 2021-02-26T02:47:11Z

src/chains/ethereum/ethereum/src/miner/miner.ts

-          storageKeys.set(hashedKey, key.toBuffer());
+          ).toBuffer();
+          const hashedKey = keccak(key);
+          storageKeys.set(hashedKey.toString(), { key, hashedKey });


Trading some memory for CPU. Previously the consumer of this data would convert the Map's keys (hashedKey.toString()) back into Buffers before saving them to the database. Now it just reads the Buffers stored in the values of the Map.

davidmurdoch · 2021-02-26T02:49:17Z

src/packages/utils/src/utils/find-insert-position.ts

+export function findInsertPosition<T>(
+  array: T[],
+  val: T,
+  comp: (a: T, b: T) => boolean
+) {
+  let count = array.length;
+
+  let first = 0;
+  let offset = 0;
+  while (count > 0) {
+    let step = (count / 2) | 0;
+    offset += step;
+    if (!comp(val, array[offset])) {
+      first = ++offset;
+      count -= step + 1;
+    } else {
+      count = step;
+      offset = first;
+    }
+  }
+  return first;


Binary searches a sorted array for the sorted insert position.

This implementation was ported from the C++ stdlib, http://www.cplusplus.com/reference/algorithm/upper_bound/.

eshaben

just a couple small things... looks good though! thanks for cleaning it up... keyStart 😆

src/chains/ethereum/ethereum/src/blockchain.ts

src/packages/utils/src/utils/find-insert-position.ts

Co-authored-by: Erin Shaben <[email protected]>

eshaben

LGTM

gnidan

Here's a couple style suggestions. This looks good!

src/packages/utils/src/utils/find-insert-position.ts

src/chains/ethereum/ethereum/src/blockchain.ts

Co-authored-by: g. nicholas d'andrea <[email protected]>

Co-authored-by: Erin Shaben <[email protected]> Co-authored-by: g. nicholas d'andrea <[email protected]>

davidmurdoch added 3 commits February 25, 2021 21:23

chore(perf): optimize debug_storageRangeAt

013adb9

cleanup

c5d60eb

remove testing things

7b40bfa

davidmurdoch commented Feb 26, 2021

View reviewed changes

davidmurdoch requested review from eshaben and gnidan February 26, 2021 02:49

davidmurdoch marked this pull request as ready for review February 26, 2021 02:49

davidmurdoch changed the title ~~chore(perf): optimize debug_storageRangeAt~~ perf: optimize debug_storageRangeAt Feb 26, 2021

eshaben reviewed Feb 26, 2021

View reviewed changes

src/chains/ethereum/ethereum/src/blockchain.ts Outdated Show resolved Hide resolved

src/chains/ethereum/ethereum/src/blockchain.ts Show resolved Hide resolved

src/packages/utils/src/utils/find-insert-position.ts Outdated Show resolved Hide resolved

davidmurdoch and others added 2 commits February 26, 2021 17:39

Update src/packages/utils/src/utils/find-insert-position.ts

02fb847

Co-authored-by: Erin Shaben <[email protected]>

chore: move RangedStorageKeys to a types file

6e68425

davidmurdoch requested review from cds-amal and eshaben February 27, 2021 00:01

eshaben approved these changes Feb 27, 2021

View reviewed changes

gnidan approved these changes Mar 3, 2021

View reviewed changes

davidmurdoch and others added 2 commits March 3, 2021 11:46

add better comments to findInsertPosition

b9c72c9

Update src/packages/utils/src/utils/find-insert-position.ts

cfce53b

Co-authored-by: g. nicholas d'andrea <[email protected]>

davidmurdoch merged commit f714114 into develop Mar 3, 2021

davidmurdoch deleted the debug_storageRangeAt-optimized branch March 3, 2021 22:44

sambacha pushed a commit to contractshark/ganache-core that referenced this pull request Apr 15, 2021

perf: optimize debug_storageRangeAt (ConsenSys-archive#809)

a006500

Co-authored-by: Erin Shaben <[email protected]> Co-authored-by: g. nicholas d'andrea <[email protected]>

		@@ -0,0 +1,11 @@
		declare module "merkle-patricia-tree/secure" {

perf: optimize debug_storageRangeAt #809

perf: optimize debug_storageRangeAt #809

Uh oh!

Conversation

davidmurdoch commented Feb 26, 2021

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

cds-amal Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

davidmurdoch Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eshaben left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eshaben left a comment

Choose a reason for hiding this comment

Uh oh!

gnidan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

perf: optimize `debug_storageRangeAt` #809

perf: optimize `debug_storageRangeAt` #809

davidmurdoch Feb 26, 2021 •

edited

Loading

davidmurdoch Feb 26, 2021 •

edited

Loading

davidmurdoch Feb 26, 2021 •

edited

Loading

davidmurdoch Feb 26, 2021 •

edited

Loading