Skip to content

Server P11b-ii: a Live selection's references are pinned once, shared and written once (G29, D68) - #167

Merged
sujitn merged 1 commit into
mainfrom
server-p11b-ii-pinned-bytes
Oct 29, 2026
Merged

sujitn merged 1 commit into
mainfrom
server-p11b-ii-pinned-bytes

Conversation

@sujitn

@sujitn sujitn commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

P11b-ii: a Live selection's references are one shared section, checked and written as JSON once per activation (G29, D68 A). The snapshot id is still SHA-256 over the same canonical bytes, so every id and every stored snapshot is unchanged.

Timers on main (temporary, not committed) put MarketSnapshot::new at 1.62 ms of the 3.4 ms one-quote tick at 256 bonds:

Stage of new µs
serde_json::to_vec of the content (999 KB) 980
SHA-256 over those bytes 398
Reference checks 156
Other checks 65
Quality 15
Normalize 3

The references are 70% of the bytes and were also cloned at every tick (0.15 ms in P11a).

Change

  • SnapshotContent.references is a SnapshotReferences: an Arc over the sorted records.
    • Readers keep their code through Deref<Target = [ResolvedBondReference]> and IntoIterator.
    • make_mut() is the only way to change the records. It copies them when shared and drops the cached JSON and checks, so neither can go stale. There is no DerefMut.
    • It serializes as the plain array, and deserialization keeps wire order, so stored JSON, to_value and json! are unchanged and verification still refuses unsorted records.
    • PartialEq and Debug read the records alone.
  • The engine pins one section per activation (PinnedInputs), and every tick's snapshot shares it instead of cloning 256 records.
  • Its canonical JSON is written once (serde_json's raw_value feature, enabled on convex-ports; no new crate, Cargo.lock unchanged).
  • MarketSnapshot::new streams the canonical JSON into the hash through a mirror of SnapshotContent that writes the references from the cached JSON. The 16 MiB bound is counted as the bytes stream.
  • The reference checks that read no cutoff run once per section. Each snapshot still runs the checks that read its cutoff, through validate_resolved_bond_at: the record was recorded by known_at, and the terms and every resource are effective at as_of and recorded by known_at. validate_resolved_bond and validate_floating_resources share the resource scope check with it, so the two cannot drift.
  • normalize sorts the references only when they are out of order, so a shared section is never copied.
  • RUNTIME.md and SNAPSHOTS.md describe the shared section.

Behaviour that changes

None. Ids, stored bytes, refusals and their errors are unchanged.

Tests

  • For schemas 1, 2 and 3, the id equals SHA-256 over serde_json::to_vec(&content), the plain writer. The id is written by the mirror and the cached JSON, so a drift in the mirror's field order or skip rules fails these tests.
  • One quote field changes the id; a reference added in place after the section's JSON was written changes it too, and it still equals the plain hash.
  • A checked, shared section is still refused at an earlier cutoff, before one of its resources was recorded. A duplicate added in place after the section was checked is refused.
  • Removing the cache resets in make_mut fails both of these tests (checked by hand).
  • The stored schema-1 and schema-2 archives (curve-replay-schema-one.json, curve-replay-schema-two.json) still deserialize and verify.

Numbers

Criterion medians, --profile ci, WSL. Two interleaved rounds against main (d25000f), swapping in its convex-engine and convex-ports sources and test support:

Bench main this PR
live_throughput/burst1/256 (one-quote tick) 3.55, 3.52 ms 2.26, 2.27 ms
live_throughput/burst512/256 29.4, 29.6 ms 25.2, 25.2 ms
snapshot/verify 17.3, 17.4 µs 12.2, 12.4 µs
snapshot_assemble/256 (Captured/Replay resolve) 4.46, 4.76 ms 4.10, 4.69 ms
snapshot/value_uncached 85.1, 85.4 µs 90.7, 88.9 µs
snapshot/value_built 73.4, 84.0 µs 82.8, 83.9 µs
live_tick stopped by a WSL clock step, 286 µs 259, 281 µs
  • The one-quote tick at 256 bonds falls by 1.25–1.29 ms (35–36%), about what the timers predicted: new's 1.62 ms and the 0.15 ms clone, less the 0.40 ms hash and the per-snapshot checks that stay.
  • snapshot/verify calls verify() again and again on one snapshot, whose section already holds its JSON and its cutoff-free checks from when it was made. So it times the cutoff checks and the streamed hash. verify has been off the Live path since P11b-i.
  • value_uncached is 4–6 µs (5%) slower in both rounds, and four more runs of this branch gave 87.9–89.4 µs. Nothing on that path uses the section beyond the binary-search lookup main also does, now one pointer further away. The cause is not found.
  • live_tick stamps raw Utc::now(), so a WSL clock step stops it (P11d).

Not done here (next P11b step)

  • P11b-iii (G30, G31, G32, G34): the merge copy of every record (0.36 ms), the drop of the previous copy (about 0.25 ms), and changed building 256 contexts (0.34 ms).
  • The full-content hash (0.40 ms) stays, as D68 A said. A sectioned identity (D68 B) is a P12 candidate.

Verification

WSL, --locked:

  • cargo fmt --check; cargo clippy --all-targets -- -D warnings for convex-ports, convex-engine, convex-server, convex-ext-redb, convex-ext-file; RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps --all-features for convex-ports and convex-engine.
  • cargo test -p convex-ports: 43 passed.
  • cargo test -p convex-engine: 207 passed, 1 ignored, as on main.
  • cargo test -p convex-ext-redb -p convex-ext-file -p convex-server -- --test-threads=1: 353 passed.
  • tools/assurance/check.py, alone and with --base d25000f5: passed.

Summary by CodeRabbit

  • Improvements
    • Snapshots that share reference data avoid repeating work that does not depend on the snapshot cutoff, while cutoff-specific checks still run for each snapshot.
    • Reference validation now checks that bonds and related resources are available and effective at the requested times.
    • Snapshot identities remain consistent with the serialized content.
  • Documentation
    • Clarified how shared references and cutoff-dependent validation work.

… and written once (G29, D68)

SnapshotContent.references is a SnapshotReferences: an Arc over the sorted
records that serializes as the plain array. The engine pins one section per
activation, every tick's snapshot shares it, and its canonical JSON is written
and its cutoff-free checks run once. MarketSnapshot::new streams the canonical
JSON into SHA-256, splicing the cached section bytes, so every id and stored
snapshot is unchanged. Each snapshot still checks the section at its own cutoff.
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 956b811f-1c60-41f9-b2d9-f449c502f23c
Commits

Reviewing files that changed from the base of the PR and between d25000f and 658cbee.

Files selected for processing: 12
  • crates/convex-engine/RUNTIME.md
  • crates/convex-engine/SNAPSHOTS.md
  • crates/convex-engine/src/snapshot.rs
  • crates/convex-engine/tests/snapshots.rs
  • crates/convex-engine/tests/support/calibration_replay.rs
  • crates/convex-ports/Cargo.toml
  • crates/convex-ports/src/reference_master.rs
  • crates/convex-ports/src/snapshot.rs
  • crates/convex-ports/tests/calibration_snapshots.rs
  • crates/convex-ports/tests/curve_recipe_snapshots.rs
  • crates/convex-ports/tests/snapshot_contracts.rs
  • crates/convex-ports/tests/support/snapshot_fixtures.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Snapshots now store resolved references in a shared SnapshotReferences section. The section caches cutoff-independent validation and canonical JSON; each snapshot still checks cutoff-dependent conditions. Snapshot identity streams canonical content through a bounded digest while preserving schema-specific hash domains.

Changes: Snapshot Reference Sharing

Layer / File(s) Summary
Reference section and validation
crates/convex-ports/src/reference_master.rs, crates/convex-ports/src/snapshot.rs, crates/convex-ports/tests/snapshot_contracts.rs, crates/convex-ports/tests/support/snapshot_fixtures.rs
SnapshotReferences stores resolved references and supports copy-on-write mutation, lookup, sorting, and validation. Reference resource checks use a shared scope check. Tests cover cutoff-specific validation and mutations.
Canonical snapshot identity
crates/convex-ports/Cargo.toml, crates/convex-ports/src/snapshot.rs, crates/convex-ports/tests/calibration_snapshots.rs, crates/convex-ports/tests/curve_recipe_snapshots.rs, crates/convex-ports/tests/snapshot_contracts.rs, crates/convex-ports/tests/support/snapshot_fixtures.rs
Identity generation streams canonical content through a bounded digest and reuses cached reference JSON. Tests compare snapshot IDs with hashes of plain serialized content and check identity changes when content changes.
Engine pinning and documentation
crates/convex-engine/src/snapshot.rs, crates/convex-engine/tests/snapshots.rs, crates/convex-engine/tests/support/calibration_replay.rs, crates/convex-engine/RUNTIME.md, crates/convex-engine/SNAPSHOTS.md
Engine snapshot construction and pinned inputs use SnapshotReferences. Tests use copy-on-write mutation. Documentation describes reference sharing and cutoff-dependent checks.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant MarketSnapshot
  participant SnapshotReferences
  participant BoundedDigest
  MarketSnapshot->>SnapshotReferences: Reuse canonical reference JSON
  MarketSnapshot->>BoundedDigest: Stream canonical content
  BoundedDigest-->>MarketSnapshot: Return schema-specific digest
Loading
Merge Risk: Minimal · up to 658cb

This change shares a Live selection's references across snapshots and streams the canonical JSON into the hash. Per the PR description, snapshot IDs, stored bytes and refusals stay the same, and the added tests check that identity matches plain serialization. No concrete merge-blocking risk was found.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and specifically describes the main change: sharing and writing a Live selection’s pinned references once. The issue identifiers add context without obscuring the change.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 9 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sujitn
sujitn merged commit d90c5ef into main Oct 29, 2026
16 checks passed
@sujitn
sujitn deleted the server-p11b-ii-pinned-bytes branch October 29, 2026 18:57
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.

1 participant