Repository navigation
[Perf][P0] open() on every pre-V4 session unconditionally scans the entire session corpus via listArtifacts() — O(N) per open, p50 1167 ms at N≈1830 #9264
Replies: 2 comments
|
One improvement I'd consider is separating subagent-child discovery from the normal session-opening path. Currently, if A few possible improvements:
I'd prioritize the index or a safe fast path before simply increasing the cache size, since a larger cache may reduce repeated work without eliminating the underlying O(N) scan. The key is to preserve migration correctness while making the cost of opening a session depend on that session's actual data, not the size of the entire corpus. Hope this helps! If this addresses the issue, would you consider marking it as the accepted answer? |
|
Thanks for reading and for the suggestions — they line up with the four in the post, and I agree with your priority ordering: an index or a safe fast path removes the scan, whereas a larger cache only reduces repeated work. On marking this as the accepted answer: I'd rather not, and the reason is worth stating so it doesn't read as dismissiveness. Nothing has been fixed yet. If you want to make it a real answer, the highest-value thing would be a measurement rather than a design: patch one of the four candidates, run the phase instrumentation before/after, and report the delta. The numbers I have are from a data-side workaround (pre-migrating the corpus to V4, which skips the discovery path entirely — Two things that might help if you do:
Happy to compare notes if you benchmark any of them. |
Uh oh!
There was an error while loading. Please reload this page.
Environment
@deepseek-ai/dsh-session-persistence-jsonl@0.2.0-rc.2(other core packages on the same line)dsh-plugin-desktop@2.0.17(DSH Desktop)Where this report comes from: I ran a performance audit on one real machine. The numbers below are all measured, not estimated. I did not modify any product source code; the "after" numbers come from a data-side workaround described at the end.
Summary
Opening one historical session costs about 1.2 s, and the cost is O(total number of sessions in the corpus) — it is not a function of the session being opened. On this machine
p50 = 1167.3 msover 1831 sessions.The cause: for any session stored below generation 4,
prepareStoredMigration()callslistArtifacts()unconditionally, which walks every session directory in the corpus and reads the first line of every zstd log, purely to answer "does this session have subagent children?". For 91.9% of sessions the answer is empty. The read path never persists the migration result, so the work repeats on every open and after every restart.After pre-migrating the corpus to V4 — no source changes — the same measurement drops to
p50 = 9.9 ms(≈118×).Root cause
packages/session/session-persistence-jsonl/lib/index.jslistArtifacts()(:3031) =ensureRootEncoding()+listGenerations()+ onereadGenerationHeader()per artifact:Three facts make this pathological:
publishStoredMigration()is called only from thewritebranch (:2495); thereadbranch (:2473–:2484) only setsstate.primed. So everyopen(id, "read")redoes the full migration, and the corpus keeps staying pre-V4.COLD_LOG_MEMO_MAX_ENTRIES = 2(:2284); it is an instance field (lost on restart);loadStoredMigrationonly hits it whenstatus === "prepared"(:2652). Two concurrently open sessions → two independent full scans with no sharing (migrationPreparationsis keyed per id).Per
dsh-session-format-v3-to-v4/README.zh.md:84— 「当前 V4 打开跳过发现」 — V4 embeds the parent catalog in the log, so V4 sessions already bypass this entire path. That is exactly why pre-migrating to V4 fixes it without touching a line of product code.Evidence
(a) Phase instrumentation —
open(id, "read")on a fresh instance, n=3 average, comparing a 1-session mini corpus with the real 1833-session corpus:ensureRootEncodingfindLoglistArtifactsrequireStoredLog(total)findLogis 4.7 ms — it is not the bottleneck. The corpus walk is.(b) Isolation experiment — monkey-patching
be.listArtifacts = async () => []in-process (zero disk changes), 8 v0/v3 samples:One sample:
1253.4 ms → 112.7 ms.(c) The scan is almost always useless — reading only the first line of all 1833 logs:
(d) Corpus-size scaling — synthetic corpora of N sessions,
openp50:N=1 → 1.2 ms · 250 → 21.3 · 500 → 43.7 · 1000 → 66.3 · 1500 → 139.6 · 1833 → 125.6normalised 6.6 – 10.3 ms per 100 sessions: strictly linear.
(e) Before/after on the real machine — 1831 sessions, real
open(), no source changes:Verified across two independent application restarts (56.2 s → 56.5 s; p50 9.9 → 9.9 ms), so it is a durable on-disk change, not a warm-cache artifact.
Reproduction
Scaling variant: build mini corpora of N = 1 / 250 / 500 / 1000 / 1833 sessions and plot
openp50 — it is linear in N, which alone proves the cost is corpus-bound rather than session-bound.Suggested fix
Disclosure: these suggestions are code-level inferences. I did NOT patch the product source and measure the result. What I verified end-to-end is the data-side workaround: pre-migrating the corpus to V4 using DSH's own
open(id, "write")(which triggerspublishStoredMigration). That is what produced the 118× number above. The changes below are proposed from reading the call graph, not from a measured patch.In rough priority order:
origin === "subagent"filter means only sessions that ever spawned subagents matter. Maintain anorigin → parentSessionindex (the header already carries both fields) instead of walking the corpus. Expected: removes ~90% of the cost for every pre-V4 open. Risk: the index must be invalidated on create/delete — same lifecycle asensureRootEncoding.publishStoredMigration()is write-only (:2495). Caching the V4 result on read (behind a "safe to materialise" check) would make the cost one-time per session instead of per open. Risk: read opens must not silently become writers — needs an explicit, auditable opt-in. This is the change most likely to have side effects.COLD_LOG_MEMO_MAX_ENTRIES = 2. With two sessions open (a common case) the memo thrashes by construction. Cheap change, modest gain, low risk.Not recommended: raising the V4 cutoff without (1) — it shifts the cost rather than removing it.
Related
This is purely a performance issue. A separate family of problems (old-format sessions being refused outright) exists in the same package and is reported separately, since the causes and the fixes are unrelated.
All reactions