Skip to content

cryptosim speedup - #4185

Open
cody-littley wants to merge 5 commits into
mainfrom
cjl/cryptosim-refactor
Open

cody-littley wants to merge 5 commits into
mainfrom
cjl/cryptosim-refactor

Conversation

@cody-littley

@cody-littley cody-littley commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Describe your changes and provide context

Optimize crytposim. At high throughput, block generation actually can become a bottleneck.

Previous cryptosim strategy was that each simulated transaction did a number of reads, then produced a number of key-value pairs that changed. This led to overhead in the benchmark framework, as managing and processing hundreds of thousands of key-value pairs adds nontrivial overhead.

In this branch, I change it so that we generate the writes for each transaction at the same time we are generating the "action plan" for the transaction. This produces equivalent data from the DB's perspective, but allows us to move the accounting work off of the main thread.

The crytposim suite is basically asking the question, "how fast can the DB go if the rest of the system is infinitely fast". In reality we will have transaction execution overhead, but that's not something that this benchmark wants to measure. So this optimization is a reasonable thing to do.

@cody-littley cody-littley self-assigned this Sep 15, 2026
@cody-littley
cody-littley marked this pull request as ready for review September 15, 2026 14:00
@cursor

cursor Bot commented Sep 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Benchmark-only changes alter workload generation, commit shape, and read paths; incorrect determinism or partitioning could skew results or break comparability across configs, but production chain code is not touched.

Overview
Speeds up cryptosim (and aligns gigasim) by moving block assembly and state writes off the execution/commit hot path, so high TPS runs are less likely to be capped by single-threaded block generation and per-transaction executor churn.

Cryptosim now builds each block with BlockBuildWorkers goroutines using forked DataGenerators and CannedRandom.SeekTo so transaction content is identical regardless of worker count. Account minting switches from NewAccountProbability to a SelectionsPerNewAccount cadence (plus deterministic hot-selection spreading) so parallel ranges can reserve account IDs in advance. The builder records KV writes into the block’s changeset (fee account once per block); executors only read the DB, ScheduleRange replaces round-robin per-txn scheduling, and Database.Get no longer short-circuits through pending writes. Commits use one EVM changeset per block instead of one per pair; sync_map is removed in favor of single-writer maps.

Gigasim gets the same measurement fix: writes are staged at block generation, execution is reads-only, stateBatch is no longer sharded/concurrent, and optional ReadCacheMetricsEnabled (default off) avoids cache metric overhead at high read rates. Tests cover worker-independence, reserved account IDs, dispatch partitioning, and read fidelity.

Reviewed by Cursor Bugbot for commit a6c92cc. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.56%. Comparing base (c45517d) to head (a6c92cc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4185      +/-   ##
==========================================
- Coverage   66.70%   65.56%   -1.14%     
==========================================
  Files        2204     2083     -121     
  Lines      169825   157877   -11948     
==========================================
- Hits       113279   103517    -9762     
+ Misses      56405    54219    -2186     
  Partials      141      141              
Flag Coverage Δ
sei-chain-pr 78.43% <100.00%> (?)
sei-db 74.50% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/common/rand/canned_random.go 78.43% <100.00%> (+10.43%) ⬆️

... and 121 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedSep 16, 2026, 8:35 AM

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d5bdbc2. Configure here.

Comment thread sei-db/bench/cryptosim/data_generator.go Outdated
Comment thread sei-db/bench/cryptosim/data_generator.go Outdated
seidroid[bot]
seidroid Bot previously requested changes Sep 15, 2026

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parallel block-build refactor is well documented and largely sound (fork-per-range randomness, range dispatch, changeset built off the critical path), but the new account-minting cadence has two correctness defects: a cadence hit that lands on a hot selection is silently dropped while its account ID stays reserved, which makes the next block re-issue IDs a fork already used; and the selection counter restarts every block, so the configured cadence is not what the benchmark actually applies.

Findings: 2 blocking | 4 non-blocking | 6 posted inline

Blockers

  • None at the file/PR level.
  • 2 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • None at the file/PR level.
  • 4 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-db/bench/cryptosim/data_generator.go Outdated
Comment thread sei-db/bench/cryptosim/data_generator.go
Comment thread sei-db/bench/cryptosim/block_builder_parallel_test.go Outdated
Comment thread sei-db/common/rand/canned_random.go Outdated
Comment thread sei-db/bench/cryptosim/data_generator.go Outdated
Comment thread sei-db/bench/cryptosim/cryptosim_config.go Outdated
@yzang2019

Copy link
Copy Markdown
Contributor

Default runs can re-issue account IDs across blocks (blocking)
RandomAccount decides creates from the cadence, then throws the mint away if the selection is hot. Meanwhile buildBlockRanges reserves IDs with AccountsMintedPerSelections, which counts every cadence hit, hot or not. AdoptForkResults then advances the parent by actual mints.

Default config is exactly the dangerous combination: HotAccountProbability: 0.1, BlockBuildWorkers: 8, cadence 1111, 1024 txns/block → two reserved IDs in different workers (selection 0 and 1111).

@cody-littley

Copy link
Copy Markdown
Contributor Author

@yzang2019

Default runs can re-issue account IDs across blocks (blocking)
RandomAccount decides creates from the cadence, then throws the mint away if the selection is hot. Meanwhile buildBlockRanges reserves IDs with AccountsMintedPerSelections, which counts every cadence hit, hot or not. AdoptForkResults then advances the parent by actual mints.

I've fixed how account creation is handled.

@cody-littley

Copy link
Copy Markdown
Contributor Author

@seidroid review

seidroid[bot]
seidroid Bot previously requested changes Sep 16, 2026

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parallel build, the generation-time changeset and the deterministic selection patterns are a coherent design, and the previous review's two blockers are genuinely fixed. One worker-count dependence remains: the cold-selection window is sized from a counter each fork mutates, so a block's contents still depend on BlockBuildWorkers once NewAccountDormancyProbability < 1, and the new invariant test's fixture cannot see it.

Findings: 1 blocking | 3 non-blocking | 4 posted inline

Blockers

  • None at the file/PR level.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • None at the file/PR level.
  • 3 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-db/bench/cryptosim/data_generator.go Outdated
Comment thread sei-db/bench/cryptosim/data_generator.go
Comment thread sei-db/bench/cryptosim/block_builder.go
Comment thread sei-db/bench/cryptosim/cryptosim_config.go
@cody-littley

Copy link
Copy Markdown
Contributor Author

@seidroid review

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parallel block build, the generation-time changeset and the "reads always reach the DB" rule are a coherent design, and last round's cold-window blocker is genuinely fixed by the coldAccountsVisibleInBlock freeze. Two non-blocking items remain: the even-spread selection pattern still puts every claimed selection on an odd count for both shipped defaults (so hot/minting selections only ever land on a transfer's destination), and the worker-independence fixture still mints on every selection, so the cold paths the new freeze protects are never exercised.

Findings: 0 blocking | 2 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

func (d *DataGenerator) selectionIsHot(selection int64) bool {
share := int64(math.Round(d.config.HotAccountProbability * selectionPatternCycle))
position := selection % selectionPatternCycle
return (position+1)*share/selectionPatternCycle > position*share/selectionPatternCycle

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Still present after the latest push, so re-reporting. The even-spread condition claims a selection only when (position+1)*share/cycle > position*share/cycle, which for an integer cycle/share puts every claimed position at p ≡ (cycle/share - 1) mod (cycle/share) — odd whenever cycle/share is even, which both shipped defaults are.

  • cryptosim, HotAccountProbability = 0.1: share = 100_000, so hot positions are p ≡ 9 (mod 10) — all odd.
  • gigasim selectionInShare (account_model.go:316), NewAccountProbability = 0.001: claimed = 1000, so mint positions are p ≡ 999 (mod 1000) — all odd; HotAccountProbability = 0.1 gives p ≡ 9 (mod 10), also all odd.

BuildTransaction / buildTransaction select source first and destination second, so a transaction's selection counts are txIndex*2 (source, even) and txIndex*2+1 (destination, odd). Consequences at the defaults:

  • cryptosim: no source account is ever hot; every hot selection is a destination.
  • gigasim: no source is ever hot and no source is ever minted, so a transfer's source is unconditionally drawn from the cold population while ~20% of destinations are hot.

The aggregate shares are preserved, so the key set the store sees is close to unchanged, but the per-transaction composition is not the one the previous per-selection draws produced — and since the source and destination reads are the ones the read caches serve, that shifts the locality this benchmark exists to measure. Nothing in the docs or tests records the new shape (TestHotSelectionsFollowTheConfiguredShare only checks the aggregate count over a contiguous run, which holds either way).

Keying the pattern on a hash of the selection count, or offsetting it by the selection's position within the transaction, restores the share on both sides while keeping it a pure function of the count — which is all the range reservation in buildBlockRanges needs.

t.Helper()
builder := newTestBuilder(t, 64)
builder.config.BlockBuildWorkers = workers
builder.config.SelectionsPerNewAccount = 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] TestBlockContentsIndependentOfWorkerCount builds off this fixture, and with SelectionsPerNewAccount = 1 on top of newTestBuilder's HotAccountProbability = 1.0 and the default NewAccountDormancyProbability = 1.0, every selection creates (creating wins the tie). So the invariant test exercises only the create branch of RandomAccount:

  • selectionIsHot's branch never runs,
  • the cold-window branch (highestSafeAccountIDInBlock / coldAccountsVisibleInBlock) never runs, and
  • isCold = rand.Float64() >= 1.0 is effectively never true, so ColdAccountsMinted() is always 0 and the coldAccountsMinted term of AdoptForkResults is never non-zero in any test.

That means the coldAccountsVisibleInBlock freeze added this round — the fix for the previous review's worker-dependence blocker — has no test that can see it: deleting the two lines in Fork/ReportEndOfBlock that hold the window still leaves the suite green. TestABlockCreatesExactlyTheReservedAccountIDs does drive the cold branch (via newMixedSelectionTestBuilder), but it only compares created addresses against the reserved run and leaves NewAccountDormancyProbability at 1.0, so neither cold selection nor cold minting is compared across worker counts.

Adding a worker-independence case with NewAccountDormancyProbability < 1, HotAccountProbability < 1 and a cadence above 1 would make all three selection kinds and the cold-mint tally live in the comparison — this is the same case requested last round.

@seidroid
seidroid Bot dismissed stale reviews from themself September 16, 2026 08:43

Superseded: latest AI review found no blocking issues.

@cody-littley
cody-littley added this pull request to the merge queue Sep 16, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants