perf(storer): remove per-chunk index lookups - #5615
Draft
gacevicljubisa wants to merge 10 commits into
Draft
gacevicljubisa wants to merge 10 commits into
gacevicljubisa wants to merge 10 commits into
Conversation
Sampling is about to start reading chunks into a buffer that each worker reuses. Nothing today checks that the bytes handed back in a SampleItem are still the bytes of that chunk, so a reused buffer would silently hand the redistribution proof the contents of some later chunk. assertValidSample now checks two things for every item: that ChunkData still reproduces ChunkAddress, and that no two items share a backing array. Every existing sample test picks both up. Also add the rulers for the work that follows. BenchmarkReserveSample1k keeps its name and behaviour so the recorded baseline stays comparable; its body moves to a helper that BenchmarkReserveSample10k reuses over a ten times larger reserve. BenchmarkChunkStoreGet measures a single chunk read, split into a variant that builds the ChunkStore handle per call as the sampler does today and one that hoists it, so the cost of the handle alone is visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq
db.ChunkStore() builds three objects every time it is called, and the sampler called it once per chunk. On a testnet node that is 2.3 million handles per round, thrown away immediately. Hoist it to one per worker. The handle is deliberately not shared across workers: the read-only chunk store makes no thread-safety promise, and three allocations per worker is already nothing next to three per chunk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq
Both ReserveSample benchmarks fill their reserve with chunk.GenerateValidRandomChunkAt, which produces content-addressed chunks only. The SOC branch of transformedAddress is therefore never measured by them, and any work that branch does beyond hashing the wrapped CAC is invisible. Benchmark the function directly, one case per chunk type, through a shim in export_test.go. The shim takes a swarm.Chunk and is held to that signature on purpose so the same benchmark source can be run against branches whose internal transformedAddress is shaped differently; only the shim changes between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq
…e sampling Accelerate ReserveSample by bypassing LevelDB index lookups through opaque storage.ChunkLocation hints without breaking clean storage abstractions: - Introduce storage.ChunkLocation and capability interfaces (LocatingPutter, LocatingReplacer, LocatingGetterInto) in pkg/storage - Support dual-format backward-compatible ChunkBinItem deserialization (106 bytes legacy, 114 bytes with location) avoiding database migrations - Encapsulate thread-safe LocationGuard inside internal/chunkstore to prevent use-after-free race conditions when Sharky reuses freed slots - Use GetIntoLoc in reserve sampler workers with automatic fallback to standard GetInto for legacy or guarded locations
…ion hints Ensure GetIntoLoc defaults to fail-safe behavior by falling back to standard verified GetInto whenever no sampling session is active: - Add LocationGuard.SessionActive() method - Check !guard.SessionActive() in GetIntoLoc before using sharky location - Update tests to verify fallback when inactive and direct read when active
Align execution order in ReplaceLoc with Delete to enforce the principle of marking resources in LocationGuard prior to releasing them in Sharky: - Move guard.MarkFreed before sh.Release in ReplaceLoc - Add test coverage verifying that old locations trigger safe indexStore fallback after ReplaceLoc
…oLoc address locking
…ore the field The location hint added to ChunkBinItem only lands on entries that are written after the upgrade. Everything already in the reserve stays in the 106-byte legacy layout, Unmarshal leaves its Location zero, and GetIntoLoc falls back to the retrieval-index lookup for every one of those chunks. The dual-format Unmarshal makes the old records readable; nothing populates them, so on an upgraded node the hint is inert until the reserve turns over on its own, which takes days or never. Measured on a 2.41M chunk testnet reserve: GetIntoLoc fell back on 100% of chunks and leveldbstore.Get stayed at 2731 B/chunk, unchanged from the branch without the hint. Add migration step 8, which walks the bin index, resolves each address in the retrieval index once, and writes the location back. Entries are flushed in windows rather than collected up front: the reserve holds millions of items and ReserveRepairer's collect-everything approach would cost hundreds of megabytes while the node is still starting. Since Location lives in the value and the key stays (Bin, BinID), rewriting does not disturb the iteration it runs inside. Each window is spread over NumCPU workers because the per-entry retrieval-index lookup is a random read and would otherwise leave the disk idle. The step is idempotent. Entries that already carry a location are skipped, and entries whose chunk is gone from the chunkstore keep a zero location so the sampler keeps falling back for them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018s5GBDGHX6Nop4RxHhsCr2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
Makes
ReserveSamplesubstantially cheaper:ChunkBinItemcarries a sharky location hint, so the per-chunk retrieval-index lookup disappears; reads fall back to the index when the hint is absent or staleMeasured
Testnet, 2.49M chunk reserve, radius 2, no SIMD, identical config, both nodes sampled simultaneously via
/rchash. Median of 3 runs.chunk_loadtransformedAddress(control — unchanged code)Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):
AI Disclosure