[Bug] sessionQuery.readEvent deep-clones the entire session on every call — reading one event is O(session size), and it locks the whole Host event loop #7356
Replies: 2 comments 1 reply
|
Thanks for the profile and the full stack — the source matches it, and the numbers say something a bit sharper than "one redundant copy". Three precision points, then the minimal fix, then what a consumer can do today and what a plugin can do in the meantime. 1. The cost is the clones, not the array
And 2. It is
|
Here it is — the raw capture from the machine that produced the profile in the original post: https://gist.github.com/Yuuyuko-uu/0f486be3d2be14e22e4f2353ae6bee5f What is in it: 16 consecutive freeze captures over ~8.5 minutes. Each capture is (1) a The first capture is the clean one: CPU, 10817 samples: 91.4% Three things that may matter when judging the patch:
Paths in the log are the stock Windows Happy to run a patched build against the same call pattern if that helps. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
sessionQuery.readEvent({ sessionId, seq })is documented and consumed as a bounded single-event read, but on the Host it resolves throughSessionCorpus.load(), which for a live session callssnapshotLive(session):So reading one event deep-clones the entire event array, synchronously.
_readEventthen uses onlyevents[seq]plus a smallbefore/afterwindow — and clones the target again:The full deep clone is pure waste for this call.
Environment
@deepseek-ai/dsh@deepseek-ai/dsh-session-querysession.v3.jsonl.zstd= 20.67 MBEvidence
CPU profile (≈17 s, 10817 samples)
Stack (identical across repeated captures during one freeze)
Impact
Any consumer that calls
readEventonce per event — a natural thing to do — becomes O(N × session size), fully synchronous, on the main thread:session/cancel, and the reload endpoint all time out, because they are allfetchon the same event loop. The UI stays on "thinking" the entire time.Why this is surprising
The consumer explicitly treats this API as bounded — its wrapper is documented as "the narrow bounded Session event reader", and the v2.2.0 release notes say "bounded asynchronous history reads instead of eager indexing". The implementation is not bounded: the cost is proportional to the whole session, not to the requested window.
Suggested fix
readEventonly needsevents[seq]plus a small window, so it should not go throughload()/snapshotLive():session.eventAt(seq)is an O(1) array index indsh-session— and slice only the requested window.Either way the "read one event" path should be O(window), not O(session).
Related reports (same family: "read one thing, pay for everything")
session-query-sqlite'sobserveSessionserializes the whole event log to compute a fingerprint (RangeError: Invalid string length); a commenter there also flags the whole-liststructuredClonecost.session/listis O(sessions) with no summary cache (2.5–3.0 s, 59.8 MB).Notes
readEventin a loop (- id: vision-router/disabled: truein the profile'scordis.patch.yml).All reactions