Jmt internal node children as a sorted slice instead of hashmap #13249

msmouse · 2024-05-10T23:00:36Z

Description

Jmt internal node children as a sorted slice instead of hashmap

binary search to look up
cheaper clone, less mem allocation

about 3% improvement on average on the single node performance test.

Type of Change

Performance improvement

Which Components or Systems Does This Change Impact?

Validator Node

How Has This Been Tested?

existing coverage

Key Areas to Review

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-05-10T23:00:40Z

⏱️ 22h 18m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-targeted-unit-tests	3h 29m	🟩 ⬜ 🟩 🟩 🟩 (+9 more)
execution-performance / single-node-performance	3h 28m	🟥 🟥 🟩 🟩 🟩 (+3 more)
rust-smoke-tests	2h 15m	🟩 ⬜ 🟩 🟩 🟩
rust-move-tests	1h 55m	🟩 ⬜ 🟩 🟩 🟩 (+9 more)
windows-build	1h 20m	🟩 🟩
forge-framework-upgrade-test / forge	1h 16m	🟩
run-tests-main-branch	1h 6m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-images / rust-all	1h 4m	⬜ 🟩 🟩 ⬜ 🟩 (+1 more)
forge-e2e-test / forge	55m	🟩 🟩 🟩 🟩
forge-compat-test / forge	53m	🟩 🟩 🟩 🟩
rust-lints	53m	🟩 🟥 🟩 🟩 🟩 (+9 more)
single-node-performance	35m	🟥
cli-e2e-tests / run-cli-tests	33m	🟥 🟩 🟥 🟥
execution-performance / test-target-determinator	25m	🟩 🟩 🟩 🟩 🟩 (+2 more)
rust-build-cached-packages	21m	🟩 🟩 🟩 🟩 🟩
test-target-determinator	20m	🟩 🟩 🟩 🟩 🟩 (+1 more)
check-dynamic-deps	20m	🟩 🟩 🟩 🟩 🟩 (+10 more)
general-lints	20m	🟩 🟩 🟩 🟩 🟩 (+9 more)
check	20m	🟩 🟩 🟩 🟩 🟩
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose	7m	🟩 🟩 🟩 🟥
semgrep/ci	6m	🟩 🟩 🟩 🟩 🟩 (+10 more)
test-target-determinator	4m	🟩
node-api-compatibility-tests / node-api-compatibility-tests	3m	🟩 🟩 🟩 🟩
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+10 more)
file_change_determinator	2m	🟩 🟩 🟩 🟩 🟩 (+9 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	50s	🟩 🟩 🟩 🟩 🟩 (+11 more)
permission-check	48s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	43s	🟩 🟩 🟩 ⬜ 🟩 (+10 more)
permission-check	42s	🟩 🟩 🟩 ⬜ 🟩 (+11 more)
permission-check	19s	🟩 🟩 🟩 🟩 🟩
determine-docker-build-metadata	11s	🟩 🟩 🟩 🟩 🟩 (+1 more)

🚨 3 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg
cli-e2e-tests / run-cli-tests	9m	7m
rust-targeted-unit-tests	23m	18m
rust-lints	5m	7m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

grao1991 · 2024-05-13T22:48:42Z

storage/jellyfish-merkle/src/restore/mod.rs

@@ -107,17 +107,20 @@ where
    /// Converts `self` to an internal node, assuming all of its children are already known and
    /// fully initialized.
    fn into_internal_node(mut self, version: Version) -> (NodeKey, InternalNode) {
-        let mut children = Children::new();
+        let mut children = Vec::new();


nit: with_capacity?

1. binary search to look up 2. cheaper clone more optimizations now that children is ordered

msmouse · 2024-05-15T04:36:07Z

Jmt internal node children as a sorted slice instead of hashmap #13249 👈
revert expected_tps for resource-groups-sender-multi-change1-kb #13283 : 1 other dependent PR (#13284 )
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @msmouse and the rest of your teammates on Graphite

github-actions · 2024-05-15T05:02:24Z

✅ Forge suite `compat` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `28076aaf382f3045543d7aa6ca13d930745682a2`

Compatibility test results for 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2 (PR)
1. Check liveness of validators at old version: 01b24e7e3548382dd25440b39a0438a993387f12
compatibility::simple-validator-upgrade::liveness-check : committed: 5290.314710740663 txn/s, latency: 5447.658429610612 ms, (p50: 4800 ms, p90: 9000 ms, p99: 11900 ms), latency samples: 233700
2. Upgrading first Validator to new version: 28076aaf382f3045543d7aa6ca13d930745682a2
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1693.9030370348698 txn/s, latency: 16665.066922036953 ms, (p50: 19300 ms, p90: 22300 ms, p99: 22800 ms), latency samples: 88760
3. Upgrading rest of first batch to new version: 28076aaf382f3045543d7aa6ca13d930745682a2
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1679.4870989772169 txn/s, latency: 15783.421120969488 ms, (p50: 19100 ms, p90: 22000 ms, p99: 25000 ms), latency samples: 92420
4. upgrading second batch to new version: 28076aaf382f3045543d7aa6ca13d930745682a2
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3534.672441574259 txn/s, latency: 8878.859875346261 ms, (p50: 9600 ms, p90: 12700 ms, p99: 13000 ms), latency samples: 144400
5. check swarm health
Compatibility test for 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2 passed
Test Ok

github-actions · 2024-05-15T05:03:00Z

✅ Forge suite `realistic_env_max_load` success on `28076aaf382f3045543d7aa6ca13d930745682a2`

two traffics test: inner traffic : committed: 8080.8678873411145 txn/s, latency: 4849.750587975977 ms, (p50: 4500 ms, p90: 5700 ms, p99: 13200 ms), latency samples: 3493340
two traffics test : committed: 100.05156256990895 txn/s, latency: 1863.8155172413792 ms, (p50: 1800 ms, p90: 2100 ms, p99: 3600 ms), latency samples: 1740
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.206, avg: 0.202", "QsPosToProposal: max: 0.344, avg: 0.239", "ConsensusProposalToOrdered: max: 0.480, avg: 0.440", "ConsensusOrderedToCommit: max: 0.377, avg: 0.367", "ConsensusProposalToCommit: max: 0.847, avg: 0.807"]
Max round gap was 1 [limit 4] at version 1731548. Max no progress secs was 6.414748 [limit 15] at version 1731548.
Test Ok

github-actions · 2024-05-15T06:05:22Z

✅ Forge suite `framework_upgrade` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `28076aaf382f3045543d7aa6ca13d930745682a2`

Compatibility test results for 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2 (PR)
Upgrade the nodes to version: 28076aaf382f3045543d7aa6ca13d930745682a2
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1150.2883897885888 txn/s, submitted: 1153.8013911698863 txn/s, failed submission: 3.5130013812974656 txn/s, expired: 3.5130013812974656 txn/s, latency: 2545.4613953044473 ms, (p50: 2100 ms, p90: 4500 ms, p99: 7500 ms), latency samples: 104780
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1268.593516786563 txn/s, submitted: 1270.2668076509883 txn/s, failed submission: 1.6732908644254645 txn/s, expired: 1.6732908644254645 txn/s, latency: 2533.740653853401 ms, (p50: 2400 ms, p90: 4000 ms, p99: 5700 ms), latency samples: 106140
5. check swarm health
Compatibility test for 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2 passed
Upgrade the remaining nodes to version: 28076aaf382f3045543d7aa6ca13d930745682a2
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1170.8030185685209 txn/s, submitted: 1173.6769685763363 txn/s, failed submission: 2.873950007815478 txn/s, expired: 2.873950007815478 txn/s, latency: 2663.390237915408 ms, (p50: 2100 ms, p90: 4500 ms, p99: 6100 ms), latency samples: 105920
Test Ok

msmouse force-pushed the 0509-alden-slice-node branch 5 times, most recently from 994ff7f to 9e6ed4d Compare May 11, 2024 00:09

msmouse changed the title ~~sample state proof verification~~ slice node May 11, 2024

msmouse force-pushed the 0509-alden-slice-node branch 3 times, most recently from 0bc653e to ae1737e Compare May 13, 2024 18:29

msmouse marked this pull request as ready for review May 13, 2024 18:30

msmouse requested review from lightmark and grao1991 as code owners May 13, 2024 18:30

msmouse requested review from areshand and zekun000 May 13, 2024 18:30

msmouse changed the title ~~slice node~~ Jmt internal node children as a sorted slice instead of hashmap May 13, 2024

zekun000 approved these changes May 13, 2024

View reviewed changes

msmouse added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label May 13, 2024

areshand approved these changes May 13, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

msmouse force-pushed the 0509-alden-slice-node branch 2 times, most recently from f8df1bf to 0b70678 Compare May 13, 2024 22:07

This comment has been minimized.

Sign in to view

grao1991 approved these changes May 13, 2024

View reviewed changes

msmouse force-pushed the 0509-alden-slice-node branch from 0b70678 to 82fbf8d Compare May 13, 2024 22:59

This comment has been minimized.

Sign in to view

msmouse enabled auto-merge (squash) May 15, 2024 04:32

msmouse added 2 commits May 15, 2024 04:35

Jmt internal node children as a sorted slice instead of hashmap

32ac325

1. binary search to look up 2. cheaper clone more optimizations now that children is ordered

calibrate single-node-performance test

28076aa

msmouse force-pushed the 0509-alden-slice-node branch from 82fbf8d to 28076aa Compare May 15, 2024 04:35

msmouse changed the base branch from main to 0514-alden-revert-expected-tps May 15, 2024 04:35

msmouse merged commit f7ce9f4 into 0514-alden-revert-expected-tps May 15, 2024
12 of 14 checks passed

msmouse deleted the 0509-alden-slice-node branch May 15, 2024 04:36

This was referenced May 15, 2024

revert expected_tps for resource-groups-sender-multi-change1-kb #13283

Merged

reverse state proof siblings #13284

Merged

partial state proofs #13274

Merged

This comment has been minimized.

Sign in to view

msmouse added a commit that referenced this pull request May 15, 2024

Jmt internal node children as a sorted slice instead of hashmap (#13249)

df57548

msmouse mentioned this pull request May 16, 2024

Fix hash_cache miss when leaf is brought up on sibling deletion #13307

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jmt internal node children as a sorted slice instead of hashmap #13249

Jmt internal node children as a sorted slice instead of hashmap #13249

msmouse commented May 10, 2024 •

edited

trunk-io bot commented May 10, 2024 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

grao1991 May 13, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

msmouse commented May 15, 2024 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

Jmt internal node children as a sorted slice instead of hashmap #13249

Jmt internal node children as a sorted slice instead of hashmap #13249

Conversation

msmouse commented May 10, 2024 • edited

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented May 10, 2024 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

grao1991 May 13, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

msmouse commented May 15, 2024 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 15, 2024

✅ Forge suite compat success on 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2

github-actions bot commented May 15, 2024

✅ Forge suite realistic_env_max_load success on 28076aaf382f3045543d7aa6ca13d930745682a2

github-actions bot commented May 15, 2024

✅ Forge suite framework_upgrade success on 01b24e7e3548382dd25440b39a0438a993387f12 ==> 28076aaf382f3045543d7aa6ca13d930745682a2

msmouse commented May 10, 2024 •

edited

trunk-io bot commented May 10, 2024 •

edited

msmouse commented May 15, 2024 •

edited

✅ Forge suite `compat` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `28076aaf382f3045543d7aa6ca13d930745682a2`

✅ Forge suite `realistic_env_max_load` success on `28076aaf382f3045543d7aa6ca13d930745682a2`

✅ Forge suite `framework_upgrade` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `28076aaf382f3045543d7aa6ca13d930745682a2`