Conversation
…de storage Design proposal for a compact, arena-based storage layer coexisting with the Node class API: 12-byte flat nodes referenced by UInt32 indices, per-symbol scratch arena with intern-copy into a frozen Sendable store, and a four-phase progressive migration plan (interop, zero-materialization read path, direct-to-arena parsing, flat serialization).
…sal 0001 Phase 1) Add a compact storage layer coexisting with the Node class API: - CompactNode: flat 12-byte node (9-bit kind ordinal + 3-bit payload kind packed into UInt16, two UInt32 payload words covering index/text/inline 1-2 children/edges range), referenced by UInt32 indices instead of pointers. - SymbolStoreBuilder: noncopyable append-only builder that hash-conses every inserted node (child-index keys, bottom-up like NodeCache.internTreeUnsafe); consuming freeze() produces an immutable Sendable SymbolStore, so single-writer and frozen-immutability are enforced by the type system. - NodeReference: 16-byte value handle with O(1) equality, mirroring Node accessors (kind/text/index/children) plus materialize()/print(using:) interop; builder.demangle(_:) bridges through a transient Node tree until Phase 3 parses directly into the arena. Measured on the 49k-symbol corpus: 201,876 unique nodes (exactly matching the NodeCache hash-consing count), 3.0 MB flat storage payload vs the <= 6 MB Phase 1 target, zero print mismatches in a 2000-symbol parity sample, build time on par with the Node path.
…Phase 2) Introduce DemanglingNode, a read-only tree protocol conformed by both Node and NodeReference, and genericize the 2179-line printer engine over it as DemanglingPrinter<Target, SomeNode>. The printer is a pure read-only consumer (it never constructs nodes), so the abstraction is clean: member names match Node's API, keeping the engine body representation-agnostic. - Public NodePrinter<Target> becomes a thin facade over DemanglingPrinter<Target, Node> — public API is unchanged. - NodeReference.print(using:) now prints straight from the SymbolStore with zero materialization instead of rebuilding a Node tree. - NodeReference.text mirrors Node.text, synthesizing the generic-parameter name for .dependentGenericParamType (the printer depends on this). - NodePrintContext.node stays a concrete Node?; the store path passes `name as? Node` (nil for NodeReference), and the String target ignores context, so rich-target semantics on the Node path are preserved. Verified byte-identical to the Node path across the full 49k-symbol corpus (SwiftUI/SwiftUICore/Foundation/stdlib/Combine) x default/simplified/ synthesizeSugar: 0 mismatches. Full dyld-cache alignment and TypeDecoder suites remain green.
…bolStore materializeNode was a naive recursive rebuild: a hash-consed subtree referenced from multiple parents materialized into duplicate Node instances, expanding the store's DAG into a tree. For symbols with heavy substitution sharing this multiplies node count and defeats the printer's per-instance memoization (printCache keys on ObjectIdentifier, so every duplicate re-renders). Add an index-keyed memo so each store index materializes once and shared subtrees stay shared (===) in the rebuilt tree. New test asserts the generic parameter subtree of $s4main1gyxxlF materializes as one shared instance and the tree equals the Node-path result.
… helpers isSimpleType, needSpaceBeforeType, isIdentifier(desired:), and isSwiftModule existed twice: as concrete members on Node and as DemanglingNode protocol-extension members. The copies were identical today, but the generic printer engine statically dispatches to the extension (extension members are not protocol requirements), so the Node copy was dead weight inside the engine and a silent-drift hazard for anything else using it. Delete the Node copies and keep the DemanglingNode extension as the single implementation; concrete-Node callers (including downstream packages) resolve to the extension with identical behavior. Mark isIdentifier/isSwiftModule @inlinable to preserve the previous inlinability, and correct the extension doc comment that wrongly claimed Node's concrete members take precedence in the engine. Also document the materialize sharing guarantee and the single-copy rule in AGENTS.md and the proposal 0001 decision log.
Genericize the tree-traversal machinery (preorder/inorder/postorder/ levelorder), the kind-lookup helpers (first(of:)/all(of:)/contains/ filter(of:)), and the identifier extraction over DemanglingNode, and conform NodeReference to Sequence with the same preorder default as Node. Store-backed consumers can now walk and classify subtrees without materializing, which is what SymbolIndexStore-style bulk indexing needs. Single-implementation rule as before: Node's Sequence conformance now rides the generic machinery, the old Element == Node sequence extension is retargeted to Element: DemanglingNode (source-compatible for concrete-Node callers), and Node.identifier moves to the shared extension. Parity test walks three symbols on both representations and asserts identical preorder/postorder kind sequences, lookups, and identifiers.
Genericize the type-decoding walk as TypeDecoderEngine<Builder, SomeNode> with the public TypeDecoder<Builder> as a source-compatible facade, plus a new NodeReference entry that decodes straight from a SymbolStore. The public TypeBuilder protocol is untouched: the five handoff points that give subtrees to the builder (createTypeDecl, createProtocolDecl, createSymbolicExtendedExistentialType, resolveOpaqueType, and the decodeMangledType callback) pass materializedNode — a new DemanglingNode requirement that is 'self' for Node (zero cost) and a sharing-preserving materialization for NodeReference. The builtinTypeName case bridges mangleAsString the same way until the Remangler is genericized. Also move hasChildren and subscript(throwChild:) into the DemanglingNode extension as single implementations (Node copies removed), and genericize TypeLookupError.init(node:). Parity test decodes six manglings through both representations with identical results.
Add mangleAsString/canMangle overloads accepting any DemanglingNode (in particular NodeReference), bridging through materializedNode. Deliberately NOT a generic remangler engine: the remangling walk constructs transient helper nodes (getUnspecialized stripping in mangleAnyNominalType/mangleBoundGenericFunction, SIL box layout wrappers in mangleSILBoxTypeWithLayout) that flow back through mangle() with shared substitution state — the same NodeFactory-backed design as the C++ Remangler. A construction-free generic engine would be a redesign of a byte-exactness-critical component for no resident-memory gain: remangling's output is a fresh String and its cost is per-call transient either way. The bridge materializes once with subtree sharing preserved. Parity test remangles four symbols through both paths with identical output.
Add intern(kind:), intern(kind:text:), intern(kind:index:), and intern(kind:children:) so index builders can construct wrapper nodes (e.g. the .type dictionary keys SymbolIndexStore-style consumers build around member contexts) straight in the arena, without a Node detour. All routes share the same hash-consing tables: a directly constructed node and an interned structurally equal Node tree collapse to one index, covered by test.
…@_spi(Internals) Deep consumers (MachOSwiftSection) need to print NodeReference trees into custom rich targets (SemanticString) and reuse the library's stack-safety wrapper when driving the engine directly. The general public surface (SymbolStore, NodeReference, DemanglingNode, traversal, print/decode/remangle entries) stays plain public; the engine-level entry points are gated behind @_spi(Internals), matching the SPI group name MachOSwiftSection already uses across its own modules. Verified from a client module: both symbols resolve under @_spi(Internals) import Demangling and are invisible without it.
…n tables (proposal 0001) Two changes eliminate the bulk-build overhead identified in Phase 1: 1. Demangler node-construction seam. All ~594 construction sites go through new createNode(...) instance methods; with internsLeaves: false (used by the new internal demangleAsNodeTransient entry that SymbolStoreBuilder.demangle now calls) they build plain uncached nodes — no NodeCache leaf writes, no global lock traffic, nothing retained after the transient tree drops. Default behavior of every public entry is unchanged. 2. Intern-table slimming. The builder's three dictionary tables (which duplicated keys: 12-byte compacts, child-index arrays, String texts) are replaced by open-addressing slot arrays holding 4-byte indices; keys are recovered from the flat buffers on comparison. Acceptance on the live dyld-cache SwiftUI corpus (234,232 symbols, debug build): 619,688 unique nodes in 8.75 MB flat storage (14.1 B/node vs <=16 target, 37 B/symbol), store build 25.3s vs interning Node path 28.5s (faster than baseline; budget allowed 1.2x slower), process footprint delta during build 9.9 MB ~= retained store + ~1 MB transient (the old scheme's high-water was ~16 MB at one fifth this corpus size). Acceptance test asserts per-unit storage and throughput budgets; cache-freedom is asserted by leaf-identity (no canonicalization across transient runs).
…ses on NodeReference Add NodeReference.textUTF8 (zero-copy ArraySlice into the store's string table) and promote isIdentifier(desired:)/isSwiftModule to DemanglingNode requirements with derived defaults, so NodeReference witnesses them by comparing string-table bytes directly — the printer's sugar-detection checks (Swift module + Optional/Array/Dictionary identifiers) no longer construct a String per call on the store path. Non-ASCII needles fall back to String comparison to preserve Unicode canonical-equivalence semantics. Parity test walks both representations and checks the witnesses and raw bytes agree.
…etion and Phase 3
…eam migration bridges The arena types carry no symbol concept — they store demangled nodes — so SymbolStore/SymbolStoreBuilder become NodeStore/NodeStoreBuilder (proposal 0001 file renamed to 0001-node-store-arena.md; NodeFactory was considered and rejected because the name is taken by the interned-singleton namespace). Companion API for the MachOSwiftSection SymbolIndexStore migration: - @_spi(Internals) demangleAsNodeTransient — classify on the transient tree before interning (returned tree is NOT canonical) - NodeReference.structurallyEquals(_ node: Node) — zero-materialization cross-representation structural equality (frozen stores drop intern tables, so Node-keyed queries bridge through this) - NodeReference: CustomStringConvertible via materialize() (debug dump) - isKind(of:) and children.second moved up to the DemanglingNode protocol extensions; concrete Node copies removed (single-implementation rule)
SwiftPM caches manifest evaluations keyed by file content hash. The main checkout and this branch's worktree share identical Package.swift bytes but resolve relative 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 0.4.3, and as the main checkout's sibling paths leaking into worktree builds). A trailing comment makes the content hashes differ.
…ruction SPI, NodeReference structural APIs - NodePrinterTarget.pushTypeReferenceScope now takes @autoclosure () -> Node?: scope-ignoring targets (String, the default) never evaluate it, keeping store-backed plain-text printing allocation-free, while rich targets evaluate materializedNode and get full type-reference scope identity on the store path. Guarded by NodePrinterScopeTests (scope sequence parity across representations). - demangleAsNodeTransient accepts a symbolicReferenceResolver; new @_spi(Internals) Node.createTransient factories bypass NodeCache for resolver-built and store-feeding construction. - NodeReference gains init(interning:) (single-tree mini store), structurallyEquals(_: NodeReference) (same-store O(1), cross-store structural walk) and structuralHash(into:) for value types keying by node structure. - Replace an invalid corpus symbol (predating NodeStore; rejected by the system demangler too) with a generated real cross-module extension initializer; all corpus literals are now validated against swift-demangle. Full suite: 410 tests / 19 suites green.
Backfills the decision log entry for 26db7a4: why the type-reference scope hook became an @autoclosure (store printing must not pay for a materialization no scope-ignoring target reads, while rich targets get the nominal subtree instead of the previous nil), why symbolic-reference resolvers needed a transient construction SPI (user-supplied resolver closures were the one construction path Phase 3's cache-free contract did not cover), what each of the three new NodeReference structural APIs is for (and why the intrinsic store-identity Hashable cannot serve as the structural hash), and the invalid corpus symbol that swift-demangle rejects as well.
The `internsLeaves: false` branch of `createNode(kind:contents:inlineChildren:)` converted `Node.Children` to an `Array`, forcing a heap allocation for 0-2 children on exactly the bulk path that inline storage exists to avoid. Forward `inlineChildren` directly, matching the interning branch right above it. Also drop the manifest cache-isolation marker comment from `Package.swift`. It worked only by making this branch's manifest text differ from main's, so it becomes self-contradictory and inert the moment it merges. Use `swift package purge-cache` for worktree cache collisions instead.
…mentation - `DemanglingPrinter`'s SPI comment claimed the type-reference scope hooks receive nil on the store path; only `NodePrintContext.node` does. Document what the hooks actually deliver, and that store-backed printing materializes a fresh subtree per evaluation — so rich targets must key scopes by structure (the remangled string) rather than by `ObjectIdentifier`/`===`. Mirror the same correction in AGENTS.md, which described it as "full type-reference scope identity". - Proposal 0001: mark Implemented (Phase 1-3; Phase 4 deferred) and correct the API sketches that no longer match the code — `@frozen`, `payloadA`/`payloadB`, `demangleAsReference`, `reference(of:)`, `text -> Substring?`, and the intern table layout. Rewrite the build-flow section: the scratch-arena design was never shipped; the cache-free transient `Node` tree is what landed, and the measurements justifying that trade-off are now stated inline. - README: document `NodeStore` under Features and add a bulk-demangling usage section covering build/freeze, `NodeReference`, and `Node` interop. - Add `Documentations/NodeStoreArena.md` as the topic document for the arena work (design, trade-offs, and Phase 1/3 measurements), and `Documentations/README.md` as the directory index.
feat(Demangling): NodeStore — arena-based compact node storage for bulk demangling (proposal 0001)
…budget `StackSafeExecutor.currentThreadHasSufficientStack` requires 2MB of *remaining* stack, while a Swift Concurrency cooperative worker and a libdispatch worker each get a 512KB stack in total. The check is therefore never satisfied off the main thread: every demangle, remangle and print created — and joined — a fresh 8MB-stack `Thread`. Measured at ~50us per call, against a ~32us demangle and a ~10us print. Two independent layers: 1. `LargeStackThreadPool` reuses long-lived 8MB workers (spun up on demand, retired after a 30s idle timeout) instead of one thread per call. Covers demangle, remangle and print alike without touching any engine. A worker runs on 8MB, so nested calls take the inline branch and never re-submit — a saturated pool cannot deadlock. 2. `executeWithinStackBudget` runs the recursion inline and falls back to a worker only when it actually nears the stack end. The print path is wired in through `DemanglingPrinter.printRootWithinStackBudget`, which probes the real stack pointer at the existing `printName` convergence point — a fixed depth threshold would have to guess per-frame size, which varies by `Target`. A partial result is discarded wholesale. `Demangler` has no single recursion convergence point and `Remangler`'s is not wired up, so both take layer 1 only. 2000 iterations on a 512KB-stack thread, before -> after: print 127.5ms -> 17.5ms (7.3x, now below the main thread) demangle 162.5ms -> 74.8ms (2.2x) 6400 demangles across 32 tasks: 162.7ms -> 99.6ms Verified against MachOSwiftSection: `MachOSwiftSectionTests` + `SwiftLayoutTests` failure sets are identical before and after (157, all pre-existing on that branch); `SwiftDumpTests` / `SwiftPrintingTests` / `MachOSymbolsTests` pass except one pre-existing snapshot failure.
Wires `Remangler` into the same optimistic-inline path the printer already uses: run on the caller's stack, fall back to a large-stack worker only when the walk actually nears the stack end. The check goes at the existing convergence point `mangle(_:depth:)`, which already guards `maxDepth` (1024, matching upstream Remangler.cpp), so coverage is complete by construction. Remangling is `throws(ManglingError)`, hence a typed-throws overload of `executeWithinStackBudget`. The two failure modes are kept distinct: a thrown error means the tree itself is malformed and propagates immediately (retrying on a worker would only reproduce it), while budget exhaustion is signalled by returning nil and is the only thing that falls back. 2000 remangles on a 512KB-stack thread: 160.0ms -> 130.1ms, i.e. 1.24x the main thread's time down to 1.00x. The gain is smaller than the printer's 7.3x purely because of body cost — print is ~10us with ~84% thread overhead, remangle is ~65us. Also records what the call-graph analysis established about `Demangler`: its main parse loop is iterative (`parseAndPushNames` over an explicit nameStack), not recursive descent, which is why upstream has no depth limit there either. Only 21 of its 160 methods sit in any cycle, none on the main loop, so it stays on thread reuse alone. Verified: full suite passes, including the dyld-cache acceptance corpus (234232 symbols, 0 store failures, 0 node-path failures).
Stack safety was a call-site convention: every print entry point had to wrap `printRoot` in `StackSafeExecutor` itself, and forgetting silently removed the protection. MachOSwiftSection's `printSemantic` did exactly that once (a deeply nested generic symbol could overflow where the identical NodeReference call would not), and when the stack budget landed it missed the gain the same way, still paying a thread hop per call. Two failures at the same spot means the responsibility sits in the wrong place. `DemanglingPrinter.print(_:options:)` now owns it, with `NodePrinter<Target>` forwarding. It is `static` by necessity: the fallback needs a pristine printer, and a `mutating printRoot` that already gave up mid-walk cannot re-run itself. The four in-library entry points collapse to one-line forwards, dropping the duplicated budgeted/fallback closure pairs. `printRoot` / `printRootWithinStackBudget` stay as low-level entries for callers that know they have headroom, now documented as such. `Node.description` keeps the plain wrapper — it walks the private `printNode` tree dump rather than the print engine. Audited every remaining `StackSafeExecutor` and `printRoot` call site in the library; the rest are fallback branches, facade forwards, or the demangle/async paths that intentionally take thread reuse only.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
Pull request overview
This PR improves performance of demangling/remangling/printing on small-stack threads by avoiding per-call thread creation, and by moving stack-safety responsibilities into the core engines so call sites can’t accidentally bypass protections.
Changes:
- Replaces “spawn an 8MB stack
Threadper call” with a reusable large-stack worker pool (LargeStackThreadPool) inStackSafeExecutor. - Adds a “budgeted inline attempt → fallback to large-stack worker” mechanism (
executeWithinStackBudget) and wires it into printing and remangling. - Updates printing entry points (
DemanglingPrinter.print,NodePrinter.print,DemanglingNode.print,NodeReference.print,Node.print) to use the new engine-owned stack-safe path; adds documentation/design notes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Demangling/Utils/StackSafeExecutor.swift | Adds reusable large-stack worker pool; adds budgeted inline execution with fallback; routes existing stack-safe execution through the pool. |
| Sources/Demangling/Store/NodeReference.swift | Routes store-backed printing through DemanglingPrinter.print (engine-owned stack safety). |
| Sources/Demangling/Store/DemanglingNode.swift | Routes protocol default print(using:) through DemanglingPrinter.print. |
| Sources/Demangling/Node/Printer/NodePrinter.swift | Implements budget-aware printing and makes stack safety an engine responsibility; adds NodePrinter.print. |
| Sources/Demangling/Node/Node+CustomStringConvertible.swift | Routes Node.print(using:) through NodePrinter.print. |
| Sources/Demangling/Main/Remangle/Remangler.swift | Adds stack-budget fields and budget-aware remangling entry point. |
| Sources/Demangling/Main/Remangle/RemangleInterface.swift | Uses executeWithinStackBudget for remangling to avoid unnecessary worker hops. |
| Documentations/StackSafeExecution.md | Adds design/measurement notes for the new approach. |
| Documentations/README.md | Links the new StackSafeExecution documentation. |
| AGENTS.md | Records the new stack-safety invariant and usage guidance for contributors. |
Comments suppressed due to low confidence (1)
Sources/Demangling/Node/Node+CustomStringConvertible.swift:21
- The doc comment for the async
print(using:)says it "always" runs on a dedicated 8MB-stackThread, but it callsStackSafeExecutor.executeAsync, which runs inline when the current thread has sufficient remaining stack. This comment is now misleading about execution behavior.
NodePrinter<String>.print(self, options: options)
}
/// Asynchronous variant of ``print(using:)``.
///
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+366
to
+370
| mutating func mangleWithinStackBudget(_ node: Node, stackFloorAddress: UInt) throws(ManglingError) -> String? { | ||
| self.stackFloorAddress = stackFloorAddress | ||
| clearBuffer() | ||
| try mangle(node, depth: 0) | ||
| return didExhaustStackBudget ? nil : buffer |
Comment on lines
+237
to
+252
| func submit(_ workItem: @escaping @Sendable () -> Void) { | ||
| condition.lock() | ||
| pendingWorkItems.append(workItem) | ||
| // Only spin up a worker when the queue outgrows the idle workers that | ||
| // are already parked on the condition; overshooting under a race just | ||
| // creates one extra worker, which then retires on its idle timeout. | ||
| let needsAdditionalWorker = pendingWorkItems.count > idleWorkerCount | ||
| condition.signal() | ||
| condition.unlock() | ||
|
|
||
| if needsAdditionalWorker { | ||
| let thread = Thread { [self] in runWorkerLoop() } | ||
| thread.stackSize = StackSafeExecutor.largeStackThreadSize | ||
| thread.qualityOfService = .userInitiated | ||
| thread.start() | ||
| } |
Mx-Iris
added a commit
that referenced
this pull request
Aug 2, 2026
…mits Third max-effort review round over this branch produced 15 findings; each was re-derived against the code, main, and the project's own decision logs. Four were real and are fixed here, two were resolved the other way by maintainer decision, and the rest are adjudicated in Documentations/KnownIssues.md so future reviews skip them instead of re-deriving. Fixed - Short-circuit kind queries were priced by path count. `first(of:)` and `contains(_:)` over a whole subtree answer "which comes first" and "is there one", so occurrence counts cannot change the result — yet evolution 0006's sweep filed them under "occurrence counts are the documented semantics", which is only true of the enumerating queries. On a doubling DAG a fruitless query cost 18.2s at 22 levels, x4 per two levels, unbounded. They now run one identity-deduped preorder pass, sharing `identifier`'s proof: a repeat visit of a shared instance cannot contain anything its first visit did not. `all(of:)`, `filter(of:)` and the `preorder()` family stay path-based. - TypeDecoder guards the sites its own childless-`.type` fix did not sweep to. `decodeTypeSequenceElement` unwrapped `.type` with a bare `children[0]` and had no depth check; the `.tuple` branch indexed past `element.children` for an empty `.tupleElement` and for one holding only a `.tupleElementName`. All seven `.type` unwrap sites in the file are now guarded. Reverted to upstream - `printExtendedExistentialTypeShape` reads children 1 and 2 again. The off-by-one is upstream's — the demangler builds them at 0 and 1 — but this port's contract is reproducing `swift demangle` byte for byte, so a consumer diffing against the toolchain must see what the toolchain sees. - Depth limits go back to 768/1024/1024. The 512/384/160 recalibration rested on a corpus scan reporting 41 levels as the deepest real symbol; downstream reported `<<too complex>>` on ordinary SwiftUI-class modules, disproving it. The debug-build stack gap those cuts addressed is real and now tracked as KnownIssues #4 — including the four assertions that had to be dropped because no depth can exercise them while the stack dies first. Also - `DemanglingPrinter.init(options:)` is internal, matching `printRoot`: it was constructible from outside the module and then undrivable. - Documentation corrected where it disagreed with the code: `Node.copy()`'s "every node" means every unique node; `NodeStore.reference(at:)` can trap, not only misresolve silently; `materializeNode`'s contract was attached to `childIndices(of:)`; the arena's "index equality is structural equality" holds only up to text spelling, since text interns byte-exact; README's `NodePrinterTarget` example was the very near-miss signature the protocol removed its defaults to reject; evolution 0001's "no breaking changes" now lists the four that shipped. - KnownIssues.md becomes a two-part adjudication record: six deferred issues (its TypeDecoder trap inventory corrected from three sites to eight) and eight findings judged false positives or deliberate design. Regression tests, all failing before the fix: two short-circuit query tests timed out at 30s (Node and NodeReference), two TypeDecoder tests killed the process with an index-out-of-range. 475 tests / 24 suites pass. Corpus: 4,573,306 symbols, zero demangle failures, zero node-tree mismatches, zero remangle mismatches. Details: evolution/0007-short-circuit-queries-and-typedecoder-sweep.md
Mx-Iris
added a commit
that referenced
this pull request
Aug 9, 2026
…ew (F14/F15 + supplementary findings) Final step of the PR #7 review handover. F14 — the structural walks resolved the shared store's locked descriptor slot once per visited node: structurallyEquals(_ node:) and structuralDigest() opened withSpans and then called store.contents(of:) / store.indexPayload(of:) inside their loops, paying a lock round-trip each and potentially reading two different views within one comparison. Both now read through a single withView resolution (the cross-reference overload already did); reference(at:) stops resolving twice for its bounds check. The atomic descriptor publish the proposal sketched is deliberately NOT built, with the reasoning in 0010's decision log: with a publish per intern, a plain-load read needs a seqlock or per-buffer 128-bit atomics with a cross-buffer tearing window, and no profile shows this lock as a hotspot now that every hot path resolves once per walk. F15 — the async print gets the same withExtendedLifetime(store) anchor as the sync variant: UnretainedNodeReference's contract puts store liveness on the calling scope, and since the retirement chain that means keeping every buffer generation alive — not something to leave to the optimizer's treatment of a closure capture. Supplementary findings, each reproduced before judging: #2 sampler stop()-without-start() no longer deadlocks on a never-signaled semaphore; #5 publishCurrentState no longer allocates an [AnyObject] literal on every intern (the keepalive anchor is three stored fields now); #8 EmbeddedFlavorTests exercises both word-table configurations and asserts they agree; #1 slotCount's zero-input infinite loop is fenced by a precondition documenting its unreachability. Verdicts for #1/#3/#6/#7/#9 recorded as KnownIssues N9-N13; #4 (SharedNodeStore lacks reference(at:)) is a real gap with no current consumer, left for the maintainer as a proposal-worthy API addition. ReviewFindingsPR7.md is now a closure index: every finding fixed or adjudicated, decision-log rows in 0004/0008/0009/0010/0011, the deferred process item and the two follow-ups (idle-machine throughput A/B, supplementary #4) recorded. Verification: full default suite 530 tests green; alignment oracle 4,573,306 symbols with unchanged counts.
Mx-Iris
added a commit
that referenced
this pull request
Aug 16, 2026
The guard was rewritten from `if printDepth > maxPrintDepth` to `guard printDepth < maxPrintDepth` when the branch moved back from the withdrawn `StackBudget` probe to a fixed limit. `printDepth` is still the *enclosing* frame count at that point — the increment is below the guard — so upstream's `if (depth > MaxDepth)` over a depth-0 root truncates on the 770th frame, while `<` truncated on the 769th. One level, but in the direction the twenty-line comment on `maxPrintDepth` argues against at length: the limit was restored from 512 to 768 precisely because downstream consumers were seeing `<<too complex>>` on ordinary generic-heavy symbols. The change was unremarked in its commit, and the comment directly above still says "Matches the C++ printer's behaviour", which it no longer did. The regression test is release-only by necessity: an unoptimized build overflows an 8MB stack somewhere around 745 `printName` frames, so exercising a 768-frame boundary in a debug test crashes the process instead of failing it (KnownIssues #4). It runs under `swift test -c release`. Found by PR #7 code review (finding 3).
Mx-Iris
added a commit
that referenced
this pull request
Sep 3, 2026
…c pipelines Evolution 0014. A task run under `withTaskExecutorPreference(StackSafeExecutor.taskExecutor)` lives on a 16MB thread, so every demangle / print / remangle inside it passes the stack probe and runs inline — the effect of `withLargeStack` extended to a whole task, which is what an async print loop needs and cannot get from a synchronous batch boundary (downstream paid one 8–21 µs hop per printed symbol). - `LargeStackTaskExecutor` (`@_spi(Internals)`, macOS 15 / iOS 18 / tvOS 18 / watchOS 11 / visionOS 2) owns a second `LargeStackThreadPool` instance: the same partitioning by QoS class, bounded growth and checked creation, but separate workers from the hop pool, so long jobs never displace the millisecond hops. - A job's QoS class is its `JobPriority` raw value (the Darwin QoS values, as the runtime's global executor files them); unspecified maps to default, unknown values are refused by the pool. Submissions use the steady-state limit only — no enqueuer blocks, so there is no wait cycle for the overflow allowance to break. A refused job falls back to a dedicated 16MB thread at the job's class, then to a global dispatch queue, never inline in `enqueue`. - `LargeStackThreadPool.init` gains `stackSize:` / `workerThreadNamePrefix:`; `spawnDedicatedLargeStackThread` gains `stackSize:` / `qualityOfServiceClass:`. - On 16MB the printer's and remangler's depth counters fire before an unoptimized stack dies (measured on the nested-Optional shape: printer 380 / remangler 200 nesting levels SIGBUS on 8MB, complete on 16MB; 383 / 260 degrade cleanly), so KnownIssues #4 is closed on the executor path for those two engines. TypeDecoder's window (~30MB needed) and the 8MB hop pool are unchanged and recorded as such. Docs: proposal 0014 (Implemented), StackSafety.md §8, KnownIssues.md #4, README, AGENTS.md. Tests: LargeStackTaskExecutorTests (10 tests); full suite 588/588 green.
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.
Problem
StackSafeExecutor.currentThreadHasSufficientStackrequires 2 MB of remaining stack, while a Swift Concurrency cooperative worker and a libdispatch worker each get a 512 KB stack in total. The check can therefore never pass off the main thread — so every demangle, every remangle and every print created a fresh 8 MB-stackThread, blocked on a semaphore, and destroyed it.Measured on this package (Apple silicon, macOS 26):
DispatchQueue.global()That is ~50 µs of thread setup per call, against a ~32 µs demangle and a ~10 µs print. Bulk work — demangling every symbol of a framework, printing every declaration of an interface — paid it per item.
Changes
1. Reuse long-lived workers (
LargeStackThreadPool). Threads are created on demand, reused while work keeps arriving, and retired after a 30 s idle timeout. Covers demangle, remangle and print alike with no engine changes. A worker runs on 8 MB, so nested calls take the inline branch and never re-submit — a saturated pool cannot deadlock.2. Optimistic inline with a stack budget (
executeWithinStackBudget). Runs the recursion on the caller's stack and falls back to a worker only when it actually nears the stack end. The budgeted attempt receives astackFloorAddress(thread stack base + 64 KB margin) and returnsnilto give up; a partial result is discarded wholesale. Both engines probe the real stack pointer at their existing convergence points (printName,mangle(_:depth:)) rather than counting frames — per-frame size varies byTargetand optimization level, so a fixed depth threshold would have to be conservative to the point of waste.3. Stack safety moved into the engine. It used to be a call-site convention: wrap
printRootinStackSafeExecutoryourself, and forgetting silently removed the protection. MachOSwiftSection'sprintSemanticdid exactly that once, and missed the budget gain the same way when it landed.DemanglingPrinter.print(_:options:)now owns it (staticby necessity — the fallback needs a pristine printer, and amutating printRootthat already gave up cannot re-run itself). The four in-library entry points collapse to one-line forwards.Results
2000 iterations on a 512 KB-stack thread:
printremangledemangleThe spread is purely body cost: print is ~10 µs with ~84% thread overhead, remangle ~65 µs, demangle ~32 µs.
Why
Demanglerkeeps thread reuse onlyIts main parse loop is not recursive descent —
parseAndPushNames()is awhileloop over an explicitnameStack, so nesting is assembled by popping the stack, not by growing the call stack. That is also why upstreamDemangler.cpphas no depth limit (thedepthparameters upstream belong toRemanglerandNodePrinter; this library already matches both at 1024 / 768).A call-graph analysis of its 160 methods found only 21 in any cycle, none on the main loop:
demangleBoundGenericArgs(depth = nested generic context levels),setParentForOpaqueReturnTypeNodesImpl↔getParentId(tree walk), and a 19-methoddemangleSwift3*component — the Swift 3 mangling is recursive descent. If that ever needs guarding, the fix is to cover those 21, not to thread a constant parameter through all 160.Verification
Full suite passes, including the dyld-cache acceptance corpus (234,232 symbols, 0 store failures, 0 node-path failures). Budgeted and unbudgeted paths were checked to produce identical output for both printing and remangling.
Design notes and measurements:
Documentations/StackSafeExecution.md.AGENTS.mdrecords the new invariant — do not go back to wrappingprintRootat the call site.