Skip to content

store: shrink root_map mem-usage - #12476

Open
kprotty-fd wants to merge 2 commits into
mainfrom
king/store-shrink-rootmap
Open

kprotty-fd wants to merge 2 commits into
mainfrom
king/store-shrink-rootmap

Conversation

@kprotty-fd

@kprotty-fd kprotty-fd commented Oct 21, 2026 •

Copy link
Copy Markdown

Folds the store's tags, shred_pool, and root_map into the same ring_pool. Combined with smaller ticket seq, use of fd_map_chain (over fd_map_chain_para) guarded under a rwlock (rserve only caller), and smaller bucket counts reduces store memory overhead.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
 ┌─ ⚡ PERF · e70f855 vs main@3a633f8 ─────────────────────────────────
 │ SUITE                               BASELINE          NEW         Δ
 │ replay tps, mainnet               12,927 tps   12,787 tps  ·  -1.09%
 │ bench tps, localnet              669,411 tps  670,314 tps  ·  +0.13%
 │ snapshot load, testnet                4.93 s       4.94 s  ·  +0.17%
+│ mem total, mainnet                150.35 GiB   148.35 GiB  ▼  -1.33%
+│ mem total, testnet                 86.66 GiB    85.66 GiB  ▼  -1.15%
 │ clean compile, firedancer             3.17 s       3.14 s  ·  -0.86%
 │ binary size, firedancer             57.92 MB     57.89 MB  ·  -0.04%
 ├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 2 IMPROVED · 5 NOISE @@
 └─────────────────────────────────────────────────────────────────────
history · 4 pushes
 ┌─ HISTORY · Δ vs main, per push, newest first ──────────────────────────────────
 │ HEAD         TPS    BENCH     SNAP    MEM·M    MEM·T  COMPILE   BINARY
+│ e70f855   -1.09%   +0.13%   +0.17%   -1.33%   -1.15%   -0.86%   -0.04%
+│ 309bc85   +0.20%   +0.10%   -0.12%   -1.33%   -1.15%   +0.42%   -0.04%
+│ dd3e08d   +0.33%   -0.48%   -5.52%   -1.33%   -1.15%   -0.17%   -0.04%
+│ d8e3670   +0.03%   +0.23%  -19.98%    0.00%    0.00%   +0.66%    0.00%
 └────────────────────────────────────────────────────────────────────────────────

@kprotty-fd
kprotty-fd force-pushed the king/store-shrink-rootmap branch from d8e3670 to dd3e08d Compare October 23, 2026 18:03
@kprotty-fd
kprotty-fd requested a review from drubin-fd October 23, 2026 18:11
@kprotty-fd
kprotty-fd marked this pull request as ready for review October 23, 2026 18:11
Copilot AI balanced review requested due to automatic review settings October 23, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Generation tags repeat on consecutive cell reuse at supported large store sizes, undermining concurrent-read validation.

1 open issue
What changed in this PR

Reduces the disk store’s index memory overhead by consolidating metadata and replacing concurrent hash maps with lock-protected chain maps.

Changes:

  • Combines tags and map entries into a shared ring pool with smaller generations and bucket arrays.
  • Reworks insertion, eviction, and queries around a shared read/write lock.
  • Updates footprint assertions and adds collision, aliasing, and slot-hint tests.
File Description
src/​disco/​store/​test_store.c Updates memory expectations and exercises ring and map edge cases.
src/​disco/​store/​fd_store.h Defines the compact layout, map types, and updated API documentation.
src/​disco/​store/​fd_store.c Implements shared-pool allocation and lock-protected disk operations.

Review effort: Balanced

Comment thread src/disco/store/fd_store.c Outdated
Copilot AI balanced review requested due to automatic review settings October 23, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The shared-memory layout and synchronization rewrite needs human review and targeted test execution to validate concurrent I/O and eviction behavior.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Folds the store's tags, shred_pool, and root_map
into the same ring_pool. Combined with smaller
ticket seq, use of fd_map_chain (over
fd_map_chain_para) guarded under an rwlock,
and smaller bucket counts reduces disk ring
index memory overhead.
guards against early ticket wrap-around for shred
rings configured above (2^30)/1280 = ~1.3TiB
by using padding bits within the ring-cell entries
as additional ticket bit storage.
@kprotty-fd
kprotty-fd force-pushed the king/store-shrink-rootmap branch from 309bc85 to e70f855 Compare October 23, 2026 22:26
Copilot AI balanced review requested due to automatic review settings October 23, 2026 22:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The shared-memory layout and synchronization redesign needs human review and runtime validation of concurrent reads, writes, and eviction.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
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.

2 participants