Feature/node store adoption - #86
Open
Mx-Iris wants to merge 5 commits into
Open
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces memory retention during/after runtime engine lifecycle events by ensuring per-image indexing state (including NodeStore-backed declaration graphs) can be released when sections or engines are torn down, and by avoiding strong retention of engines from abandoned connection probes.
Changes:
- Make the injected-peer handshake probe task capture
RuntimeEngineweakly to avoid unnecessarily pinning engines via long-lived tasks. - Explicitly detach per-image Swift/ObjC indexers from their aggregate indexers on section removal (and on “remove all”), allowing declaration graphs/NodeStores to be reclaimed.
- Explicitly release all indexed sections when a
RuntimeEngineis stopped to bound memory even if something temporarily retains the engine instance.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Engine/RuntimeEngineManager.swift | Avoid strong engine retention in abandoned peer-probe tasks during handshake polling. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/RuntimeEngine.swift | Release all indexed sections during stop() to bound retained indexing memory. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Indexing/RuntimeSwiftInterfaceIndexer.swift | Add inverse API to detach per-image Swift indexers from the aggregate indexer. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Indexing/RuntimeObjCInterfaceIndexer.swift | Add inverse API to detach per-image ObjC indexers from the aggregate indexer. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Core/RuntimeSwiftSection.swift | Materialize nodes for specialization resolution; detach Swift per-image indexers on section removal. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Core/RuntimeObjCSection.swift | Detach ObjC per-image indexers on section removal so indexing state can be reclaimed. |
| RuntimeViewerCore/Package.swift | Add a manifest-cache note to prevent cross-worktree SwiftPM manifest cache collisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mx-Iris
force-pushed
the
feature/node-store-adoption
branch
3 times, most recently
from
August 4, 2026 09:21
f15b59e to
b5cd042
Compare
Mx-Iris
force-pushed
the
feature/node-store-adoption
branch
from
August 5, 2026 02:42
b5cd042 to
14fb46c
Compare
SwiftPM caches manifest evaluations keyed by file content hash. The main checkout and this worktree share identical Package.swift bytes but resolve ../../ local-dependency candidates to different paths, so either side could be served the other's cached evaluation (observed as local path dependencies silently degrading to remote tags mid-session). A trailing comment makes the content hashes differ.
…alization node building MachOSwiftSection's declaration values now hold NodeReference (Stage 5a); the two tree-building sites materialize the small type-name subtrees on demand. All mangleAsString call sites pass through the generic DemanglingNode bridge unchanged.
…ne's lifetime `removeSection(for:)` dropped its `sections` entry and reclaimed nothing: the per-image indexer is also registered with the aggregate via `setupForFactory` → `indexer.addSubIndexer(...)`, and *that* reference is what keeps the image's declaration graph — including the `NodeStore` its definitions hold — resident. The aggregate lives as long as its factory, i.e. as long as the owning `RuntimeEngine`, and `addSubIndexer` had no inverse at all, so per-image state accumulated monotonically while browsing and could never be released. - Add `removeSubIndexer(_:)` to `RuntimeSwiftInterfaceIndexer` (undoing both the local registration and the upstream one) and to `RuntimeObjCInterfaceIndexer`. - Detach the sub-indexer in both factories' `removeSection` / `removeAllSections` before dropping the entry, so those methods now actually free memory. - Call the teardown from `RuntimeEngine.stop()`. A stopped engine is usually deallocated right after, which would free this anyway — but anything that outlives the stop while holding the engine (an abandoned probe task, a suspended request) would otherwise keep the full indexed graph of every image the user ever opened resident. Releasing explicitly bounds that to the engine object itself. - Capture `engine` weakly in `pollUntilPeerAnswers`'s probe task. The function deliberately abandons rather than awaits a stuck probe, and `cancel()` cannot interrupt an XPC send that ignores cancellation, so a strong capture pinned the engine indefinitely. The engine stays alive across a normal poll because the caller awaits the function. Deliberately NOT changed: `RuntimeMessageChannel`'s optional request timeout. `finishReceiving` already drains every pending request and resumes each with an error, so a dead peer unblocks its awaiters on channel teardown; the only remaining hang needs a healthy channel plus a silent peer, and it holds small Codable values rather than any index state. Forcing a default timeout would break legitimately long forwarded operations for no memory benefit. Verified by building RuntimeViewerCore and RuntimeViewerPackages; the runtime effect (memory actually dropping after an engine stops) still wants a real memory-graph check in the app.
Mx-Iris
force-pushed
the
feature/node-store-adoption
branch
from
August 6, 2026 04:08
14fb46c to
f41648a
Compare
MachOSwiftSection's memory work (its evolution 0002) slims TypeDefinition and ExtensionDefinition down to descriptor-backed storage: the eagerly materialized TypeContextWrapper stored property is gone and typeContextDescriptorWrapper is exposed directly on the definition. Update the eight consuming sites (generic-flag checks, class hierarchy dumping, specialization request building, and the interface indexer) to the direct accessor; every call is mechanical, no behavior change. Requires a MachOSwiftSection release that includes that work — the exact: pin in RuntimeViewerCore/Package.swift must be bumped alongside merging this, since 0.14.1 still ships the stored-wrapper shape.
demangleAsNode interns every node it produces into swift-demangling's global NodeStore, which never evicts — so demangling a symbol just to read one identifier or to immediately remangle it keeps the whole tree resident for the rest of the process. Both such call sites (the ObjC reference probe in RuntimeEngine and the stable-Swift-class bridge in RuntimeRelationshipsResolver) now use demangleAsNodeTransient. A transient tree is not canonical, but remangling stays sound: the remangler's substitution table compares nodes structurally (SubstitutionEntry uses deepEquals), with identity only as a memoization fast path, so the mangled output stays in the same key space as makeRuntimeObject(forMangledTypeName:). Requires a swift-demangling release with the transient API (present on its main since the node-store work merged); the MachOSwiftSection pin bump that accompanies this branch's merge brings it in.
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
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.
No description provided.