Repository navigation
perf(storage): artifact metadata rewrites the full record table on every mutation (O(M×N) during startup) #4037
Description
Activity
Heads-up: this proposal is likely superseded by the retirement direction in discussion #4030. That discussion has converged on retiring the Artifact authority entirely, and its stated cutover invariants already subsume this issue ("surviving metadata stores must use indexed row operations rather than
readAll/replaceAll"). If retirement proceeds on that timeline, a contract-level change to the currentArtifactStoremetadata layer is probably not worth the review investment. I'm repurposing this issue as a fallback stopgap option in case the retirement takes longer than expected — happy to close it if maintainers prefer to track everything under the retirement work.This comment was prepared with AI assistance (Kimi k3-256k).
- added 9 commits that reference this issue
on Sep 4, 2026 12 remaining items
- added a commit that references this issue
on Sep 4, 2026 - No description provided.
Reopening: #4716 landed only half of this issue.
The Mechanism section above describes two costs per mutation. #4716 replaced step 2 (
replaceAll→ change-trackedINSERT ... ON CONFLICT/DELETE), andartifactIdentityKeyis no longer re-hashed for unchanged rows. That part is done and should stay closed.Step 1 is untouched.
ArtifactStore.load()andreloadForMutationUnlocked()both still callmetadataRepository.readAll()— a fullSELECT record_jsonplusdecodeArtifactRecordJsons()over all N rows — on every read path and again before every mutation. In the profile above that is the ~6.5% decode + ~5.4% SELECT share, and unlike the write side it has no M multiplier to remove: it is paid per operation in steady state too.The open design question is Question 2 in the description, which was never answered: does the in-memory record map stay authoritative (and something has to define when it is invalidated), or does
ArtifactStorequery SQLite by session/id through the indexes and stop holding the full set? The second reading matches thereadAll/replaceAllretirement invariant from #4030. This issue is the tracker for that decision — #4030 is closed and lists #4037 as one of its four execution threads, so the "likely superseded" note above no longer applies.take
@lhpqaq thanks for taking this — two things that may save you a round trip.
Question 2 in the body has an answer already on record. #4030 states the cutover invariant as "surviving metadata stores must use indexed row operations rather than
readAll/replaceAll". That settles the direction: query SQLite by index, stop holding the full table in memory. The indexes are already there (artifact_idprimary key, plus(session_id, created_at, artifact_id)).One seam worth planning for before you start. Revision is computed from the whole session.
sessionSnapshot()(packages/storage/src/artifact-store.ts:851) filters, sorts, and hashes every record in the session, andgetInSessionandlistPageboth return it. So replacingreadAll()with a per-Session query, on its own, still decodes an entire Session to answer a single-record read — the N just changes what it counts. Either revision becomes a value maintained inside the metadata transaction, or its cost needs to be stated explicitly.A useful acceptance check: adding records to other Sessions must not change the work done by a read, and adding records to the target Session must not change the work done by a single-record read — including obtaining its revision.
- added 6 commits that reference this issue
on Sep 5, 2026 @Astro-Han Thanks for the clarification. I've used indexed queries and transaction-maintained Session revisions, with tests covering both growth checks. The implementation and verification details are in #4874 — feedback welcome.
- added a commit that references this issue
on Sep 15, 2026
Problem
Every artifact metadata mutation rewrites the entire
artifact_recordstable, so a store with N records pays O(N) JSON + SQL + hashing work per mutation — and startup performs M serialized mutations (session-retirement purges, recovery adoptions), making cold start O(M×N).Mechanism in
packages/storage/src/artifact-store.ts+sqlite-artifact-metadata.ts:reloadForMutationUnlocked()→metadataRepository.readAll():SELECT record_json FROM artifact_records+decodeArtifactRecordJsons()over all N rows.metadataRepository.replaceAll(records):DELETE FROM artifact_records, then re-INSERT all N rows, each with a freshJSON.stringify(record)and a fresh sha256artifactIdentityKey(record.id).Measured impact
On a real store with ~11.7k artifact records and hundreds of accumulated Sessions (details in this #4027 comment), a windowed CPU profile of Runtime Host cold start shows the first ~90 s of the ~130 s total saturated by exactly this churn: ~34% of samples in the per-record INSERT, ~11% in the
DELETE, ~6.5% in JSON decode of the full read-back, ~5.4% in theSELECT ... all(), and ~5.6% in sha256 (identity keys are re-hashed for every record on every rewrite). After ~105 s the Host is idle; the metadata phase is the residual cold-start bottleneck.Steady state pays the same O(N) cost on every single-artifact mutation (create / purge / adopt), just without the M multiplier.
Proposed direction
Change-tracked write-back in
replaceAll's place: within the same single write transaction, apply only the delta the mutation actually made — targetedINSERT ... ON CONFLICT(storage_key) DO UPDATE/DELETE WHERE storage_key IN (...)— instead of deleting and re-inserting the full snapshot. Additionally, compute the sha256artifactIdentityKeyonce per record (it is a pure function of the record id) rather than once per record per rewrite.Combined with a batched multi-Session purge (separate proposal), this reduces a retirement batch of M Sessions from M full-table rewrites to one O(changed) commit.
Questions for maintainers
replaceAllpersists the post-mutation snapshot as one atomic transaction. Targeted writes would run inside the same single write transaction, so atomicity should be equivalent — is there any consumer that relies on the table being a full rewrite (e.g. for recovery or consistency checking) rather than just "the persisted state after transaction commit"?Related: #4027 (cold-start investigation), #4031 (bounded purge resolution; review thread identified the O(M×N) structure).
This issue was prepared with AI assistance (Kimi k3-256k), including profiling and analysis.