Skip to content
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

Make LayeredMap address leaves by the hash value of the key #14021

Merged
merged 5 commits into from
Jul 18, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Jul 16, 2024

Description

  1. no longer require the key's binary format to be unique and
    prefix-free
  2. no longer require key's binary format to be in a uniform
    distribution and doesn't require the keys to start differ
    "early" (at the first bits) to be efficient

all we require now is the key is hashable, the tree will be organized by
the hash value bits instead of the key bits

Type of Change

  • New feature

Which Components or Systems Does This Change Impact?

  • Validator Node

How Has This Been Tested?

unit tests
and rebase my experiments (of inc hash and no hash) on top of it

Copy link

trunk-io bot commented Jul 16, 2024

⏱️ 10h 29m total CI duration on this PR
Job Cumulative Duration Recent Runs
test-fuzzers 7h 46m 🟩🟩🟩🟩🟩 (+7 more)
general-lints 20m 🟩🟩🟩 (+7 more)
rust-cargo-deny 20m 🟩🟩🟩 (+7 more)
forge-e2e-test / forge 16m 🟩
check-dynamic-deps 15m 🟩🟩🟩🟩🟩 (+7 more)
forge-compat-test / forge 12m 🟩
execution-performance / test-target-determinator 8m 🟩
test-target-determinator 8m 🟩
rust-move-tests 7m 🟩
rust-move-tests 6m 🟩
rust-move-tests 6m 🟩
rust-move-tests 6m 🟩
semgrep/ci 5m 🟩🟩🟩🟩🟩 (+7 more)
check 4m 🟩
rust-move-tests 3m 🟩
rust-move-tests 3m 🟩
rust-move-tests 3m 🟩
rust-move-tests 3m 🟩
rust-move-tests 3m 🟩
rust-move-tests 3m 🟥
file_change_determinator 3m 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+7 more)
adhoc-forge-test / forge 2m 🟥
rust-move-tests 2m
permission-check 45s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 40s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 39s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 37s 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 11s 🟩
rust-move-tests 10s
execution-performance / single-node-performance 9s 🟩
determine-docker-build-metadata 7s 🟩
permission-check 7s 🟩
forge-framework-upgrade-test / forge 6s 🟩
permission-check 3s 🟩
Backport PR 3s 🟥
determine-forge-run-metadata 1s 🟩

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

Job Duration vs 7d avg Delta
test-target-determinator 8m 5m +59%
execution-performance / test-target-determinator 8m 5m +50%
test-fuzzers 46m 38m +21%
forge-framework-upgrade-test / forge 6s 8m -99%
execution-performance / single-node-performance 9s 13m -99%

settingsfeedbackdocs ⋅ learn more about trunk.io

@msmouse msmouse force-pushed the 0711-alden-layered-hash-map branch 5 times, most recently from 9557ac9 to 2b541ca Compare July 16, 2024 23:14
@msmouse msmouse changed the title LayeredMap: carries Value instead of Option<Value> Make LayeredMap a hash map Jul 16, 2024
@msmouse msmouse force-pushed the 0711-alden-layered-hash-map branch 3 times, most recently from 609cf8b to cab6410 Compare July 17, 2024 00:35
@msmouse msmouse marked this pull request as ready for review July 17, 2024 00:35
@msmouse msmouse force-pushed the 0711-alden-layered-hash-map branch from cab6410 to f27dc7d Compare July 17, 2024 18:11
@msmouse msmouse force-pushed the 0711-alden-layered-hash-map branch from f27dc7d to ea0b249 Compare July 18, 2024 20:14
@msmouse msmouse changed the title Make LayeredMap a hash map Make LayeredMap address leaves by the hash value of the key Jul 18, 2024
msmouse added 5 commits July 18, 2024 20:18
1. no longer require the key's binary format to be unique and
   prefix-free
2. no longer require key's binary format to be in a uniform
   distribution and doesn't require the keys to start differ
   "early" (at the first bits) to be efficient

all we require now is the key is hashable, the tree will be organized by
the hash value bits instead of the key bits
@msmouse msmouse force-pushed the 0711-alden-layered-hash-map branch from ea0b249 to 7705a41 Compare July 18, 2024 20:20
@msmouse msmouse enabled auto-merge (squash) July 18, 2024 20:21

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7705a41986876b40288120b3b2410e2ee05363ab

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7705a41986876b40288120b3b2410e2ee05363ab (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 6446.431486110251 txn/s, latency: 5021.867871317356 ms, (p50: 4200 ms, p90: 6700 ms, p99: 18100 ms), latency samples: 262040
2. Upgrading first Validator to new version: 7705a41986876b40288120b3b2410e2ee05363ab
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 1879.2059156879468 txn/s, latency: 13399.329093514869 ms, (p50: 16100 ms, p90: 21600 ms, p99: 24600 ms), latency samples: 55820
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6689.076609386234 txn/s, latency: 4502.711817636473 ms, (p50: 4400 ms, p90: 6800 ms, p99: 7300 ms), latency samples: 233380
3. Upgrading rest of first batch to new version: 7705a41986876b40288120b3b2410e2ee05363ab
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6530.266851961485 txn/s, latency: 4156.086014781125 ms, (p50: 4200 ms, p90: 5800 ms, p99: 6000 ms), latency samples: 140720
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6404.549463727892 txn/s, latency: 4799.610238665602 ms, (p50: 4800 ms, p90: 7100 ms, p99: 7500 ms), latency samples: 225420
4. upgrading second batch to new version: 7705a41986876b40288120b3b2410e2ee05363ab
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 10123.355894140097 txn/s, latency: 2761.221593131928 ms, (p50: 2900 ms, p90: 3700 ms, p99: 4400 ms), latency samples: 184040
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10521.076673653426 txn/s, latency: 3292.2189321772858 ms, (p50: 3100 ms, p90: 4400 ms, p99: 6900 ms), latency samples: 350620
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7705a41986876b40288120b3b2410e2ee05363ab passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 7705a41986876b40288120b3b2410e2ee05363ab

two traffics test: inner traffic : committed: 9150.470826712544 txn/s, submitted: 9316.87362769498 txn/s, failed submission: 34.50090000454562 txn/s, expired: 166.40280098243645 txn/s, latency: 5335.887138459593 ms, (p50: 4200 ms, p90: 9100 ms, p99: 18700 ms), latency samples: 3479210
two traffics test : committed: 100.00407962517713 txn/s, latency: 2083.6904040404042 ms, (p50: 2100 ms, p90: 2400 ms, p99: 5800 ms), latency samples: 1980
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.240, avg: 0.217", "QsPosToProposal: max: 1.324, avg: 0.863", "ConsensusProposalToOrdered: max: 0.322, avg: 0.298", "ConsensusOrderedToCommit: max: 0.417, avg: 0.399", "ConsensusProposalToCommit: max: 0.715, avg: 0.697"]
Max round gap was 1 [limit 4] at version 1340468. Max no progress secs was 5.77502 [limit 15] at version 1340468.
Test Ok

@msmouse msmouse merged commit ce05665 into main Jul 18, 2024
86 of 92 checks passed
@msmouse msmouse deleted the 0711-alden-layered-hash-map branch July 18, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants