fix: reduce renderer memory accumulation - #8759
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewNo outstanding issues remain. All previously flagged items have been resolved. Resolved Issues (full cycle)
Files Reviewed
Reviewed by claude-4.6-sonnet-20260217 · 954,963 tokens Review guidance: REVIEW.md from base branch |
|
@IamCoder18 did you actually profile this? |
7c4c780 to
4106a14
Compare
460ed7f to
69e353f
Compare
c122550 to
293dcca
Compare
|
@marius-kilocode I profiled my changes by switching between 10 memory-intensive sessions.
|
3a5ebf6 to
b13862a
Compare
b13862a to
93a79b9
Compare
12424fc to
1c97731
Compare
|
Hi @marius-kilocode, I’ve just pushed a few updates to this PR. I’ve rebased against I fixed an issue where the scroll location would jump when new messages were loaded. I had also resolved the issues you pointed out earlier and had replied to each of them. Whenever you have a moment, I’d appreciate it if you could take another look at the changes! |
|
The change and optimization might work, but the changes are risky, this is what my bot found: High: queued prompts can render twice or get the wrong queued state in MessageList. See packages/kilo-vscode/webview-ui/src/components/chat/MessageList.tsx:321 and packages/kilo-vscode/webview-ui/src/components/chat/MessageList.tsx:342. High: switching away from a session deletes its transcript cache but does not clear the “loaded” marker, so switching back can produce empty or stale transcript behavior. See packages/kilo-vscode/webview-ui/src/context/session.tsx:2082. High: switching sessions now deletes unsent prompt drafts, including review/image draft state, which is a user-visible regression. See packages/kilo-vscode/webview-ui/src/context/session.tsx:2116. |
|
Can you please re-validate the current head against these cases?
Please validate session switching in both places:
Please say which surface the memory profile was run against. Thanks, I want to make sure the latest force-pushed version still has the memory win without regressing core chat behavior. |
9cae484 to
16993a2
Compare
345c859 to
b5f27ad
Compare
b5f27ad to
d733bc1
Compare
|
Hey! Thank you so much for your detailed feedback! I essentially re-applied the core changes on main and resolved the issues pointed out. Here are my runs (all with the sidebar): Draft with Image Survivesdraftwithimage.mp4Switching between two sessions, no empty listloadingagainanddraft.mp4Queued Messagesloadingagainanddraft.mp4The heap snapshots didn't show much of a change. I may be doing it wrong since I don't have much experience with memory related changes. If I am, I would appreciate it if you could guide me through properly identifying improvement. Here are my heap snapshots (run against the sidebar) anyway, if it helps: Before my changesheapsnapshotsbefore.mp4After my changesheapsnapshotsafter.mp4Lastly, I'm going to open a PR in the cloud repo to mirror the new config value as per the contributing guidelines. |
|
Created the PR in the cloud repo to mirror the config value: Kilo-Org/cloud#3408 |
|
@marius-kilocode I think this is ready to review. |
|
Found a current P1 issue that should be fixed before merge:
Suggested fixes:
This preserves the memory-cache cleanup goal while avoiding active user data loss on ordinary session navigation. |
Thanks for the detailed analysis! However, after tracing every code path you described , the described scenario cannot occur in the current codebase. Here's why:
This was shown in the video, which I am showing here again: draftwithimage.mp4However, this used to be an issue previously that I had resolved. |
This change addresses OOM crashes by reducing memory usage in the webview renderer process: - chore(webview): add message list virtualization with initial load limiting - perf(session): implement aggressive session cleanup on switch and deletion - perf(server): strip heavy metadata (file diffs/tool output) from SSE events - refactor(metadata): add slim parsers for websearch, webfetch, codesearch, and task tools - feat(settings): add auto-expand history toggle to UI
d733bc1 to
28664c0
Compare
| if (oldID && oldID !== id) { | ||
| const msgs = store.messages[oldID] ?? [] | ||
| const msgIds = msgs.map((m) => m.id) | ||
| queueMicrotask(() => { |
There was a problem hiding this comment.
WARNING: Race condition with rapid session switching (A→B→A)
The queueMicrotask captures oldID but runs after selectSession returns. If the user switches A→B then quickly back to A before the microtask fires:
- A→B:
oldID=A, microtask queued to delete A's store data,currentSessionID = B - B→A:
ready = loaded().has("A") === true(microtask hasn't fired yet),setCurrentSessionID("A"),mode: "focus"sent - Microtask 1 fires: deletes A's
messages,parts,todos, etc. from store → the active session A now shows empty - Reconcile from
mode: "focus"eventually repopulates, but there's a visible blank flash
A guard inside the microtask would prevent this: skip the delete if currentSessionID() has already switched back to oldID.
queueMicrotask(() => {
if (currentSessionID() === oldID) return // switched back — don't evict
...
})|
The Cloud PR (Kilo-Org/cloud#3489) adding the config change is merged, so these changes can be merged without risking invalid config issues! |
|
I actually attempted to resolve merge conflicts, but that cause some issues. Then I tried to undo that commit, but it caused a revert commit that reset all changes from this branch's base. I'll try to fix it. Sorry about that. |
6a16f20 to
2e571b0
Compare
|
Force-pushed to fix it. |
|
@IamCoder18 I will have another look there, are a few things that are still risky with this PR. Which is why I haven't merged it. Sorry about this, but this is really hard to trace back later. |
|
@IamCoder18 can you still reproduce memory issues in the current main? We rewrote the whole session rendering to use virtualized bounded transcript rows (see: #11094) mainly for performance reasons and less for memory. But it should also mean that we don't accumulate memory anymore. |
|
We really appreciate the effort you put into this. Looking at the PR we probably should not integrate anymore:
What still makes sense:
Would you like me to extract those into separate PR's or do you prefer to do this yourself? |
Yeah I can do this later today or tomorrow. |
|
Made the first PR to resolve the unsent drafts: #11442 |
|
Closing this as we follow up on this, thanks @IamCoder18! |

Context
The VS Code extension's webview (renderer process) has several patterns that cause unbounded memory growth during normal usage. With Electron's ~4GB V8 heap limit, a 3-hour active session with moderate tool usage can approach this ceiling and trigger the "grey screen" OOM crash.
Closes #8607
Implementation
1. Message List Virtualization (Webview)
turnInit = 10/turnBatch = 8windowing in MessageList.tsx2. Session Cleanup (Webview)
handleSessionsLoaded()now cleans up orphaned messages, parts, todos, agent selections, drafts, and pending optimistic messages when sessions are removed from the indexselectSession()cleans up old session data when switching awayhandleSessionDeleted()callsclearSessionDrafts()3. SSE Event Stripping (Server)
session/index.ts: ApplystripPartMetadata()before publishingPartUpdatedSSE events (removes filediff.before/after)session/summary.ts: Stripbefore/afterfrom diff events before publishing4. Metadata Slimming (Extension)
slimPartto cover more tool types:webfetch,websearch,codesearch,read,grep,tasksessionIdin task metadata for sub-agent linkingHow to Test
bun run extensionGet in Touch
My discord is
@iamcoder18and I am in the Kilo discord server.