From b6248a0b80461e5182d2c96e259b767c9587e8f8 Mon Sep 17 00:00:00 2001 From: araq Date: Mon, 31 Aug 2026 00:44:23 +0200 Subject: [PATCH] IC: read an exported symbol's kind from its header, not by decoding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `addReexportedEnumFields` forced every exported symbol through `loadSym` and THEN asked whether it was a non-pure enum type. Almost none are: 34815 symbols on a 68-module build, of which the enum handling wants the handful that are types. A sym def is written `(sd …)`, so the kind is three tokens in and needs no decode at all. addReexportedEnumFields 290ms -> 5ms export branch 484ms -> 154ms processTopLevel 1086ms -> 787ms loadDepClosure 2126ms -> 1978ms cold --ic:on build 8.69s -> 8.51s (baseline built alongside) `peekSymKind` mirrors `loadSymFromCursor`'s walk and the two have to change together, so it is graded rather than trusted. `-d:icPeekKindCheck` compares every peek against the load it replaces: a full build is 34508 peeks, zero disagreements, and sabotaging the peek to answer `skProc` where the def says `skType` fires on the first symbol. The FIRST sabotage did not fire, and that is the part worth recording. Dropping a `skip` from the walk lands on a non-`TagLit`, which answers `skUnknown` — the designed fallback, correct but slower — so the equality assertion never saw it. A walk that had drifted out of step would therefore look exactly like a clean run. So the check has a second half: `PeekFallback` counts how often the peek cannot read the header and `-d:icBNodeProf` reports it beside `PeekKind`. It is 0, which is the claim that the walk is in step; an equality oracle alone could not make it. Verified: both configurations build; `tests/ic` 40/40; 67/67 generated `.c` byte-identical, cursor still identical to `PNode`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE --- compiler/ast2nif.nim | 63 ++++++++++++++++++++++++++++++++++++++++++-- compiler/icprof.nim | 2 +- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index c517aef48a..75cc121359 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -3977,12 +3977,71 @@ proc nifModuleHasIncludes*(config: ConfigRef; fileIdx: FileIndex): bool = done = true skip c -proc addReexportedEnumFields(c: var DecodeContext; sym: PSym; interf: var TStrTable) = +proc peekSymKind(c: var DecodeContext; module: FileIndex; + entry: NifIndexEntry): TSymKind = + ## The kind a symbol's `(sd …)` header records, WITHOUT decoding the symbol. + ## + ## The layout is `(sd …)`, which is + ## exactly what `loadSymFromCursor` walks — that proc is the definition this + ## mirrors, so the two must be changed together. Anything unexpected answers + ## `skUnknown` and the caller falls back to a real load rather than guessing. + var n = cursorFromIndexEntry(c, module, entry) + if n.kind != TagLit or not tagIs(n, symDefTagName): return skUnknown + var k = childCursor(n) + if not k.hasMore or k.kind != SymbolDef: return skUnknown + skip k # the name + if not k.hasMore: return skUnknown + skip k # the `x` / `.` export marker + if not k.hasMore or k.kind != TagLit: return skUnknown + result = parse(TSymKind, cursorTag(k)) + +proc symKindFast(c: var DecodeContext; sym: PSym; symAsStr: string): TSymKind = + ## `sym`'s kind, taken from its def header while it is still `Partial` rather + ## than by forcing the full decode. An already-loaded symbol answers from the + ## field, and anything the peek cannot read falls back to loading. + ## + ## `-d:icPeekKindCheck` grades the peek against the load it replaces, on every + ## call: the loaded kind is authoritative, so a disagreement is the peek's bug. + ## The oracle has to be run for the answer to mean anything — and broken on + ## purpose once, to confirm it fires. + if sym.state != Partial: + prof pPeekLoaded + return sym.kindImpl + let e = c.syms.getOrDefault(symAsStr) + if e[1].offset == 0: + prof pPeekFallback + loadSym(c, sym) + return sym.kindImpl + result = peekSymKind(c, sym.itemId.module.FileIndex, e[1]) + if result == skUnknown: + # The peek could not read the header. Correct, but it is also how a walk + # that has drifted out of step with `loadSymFromCursor` would present, so + # the rate is counted rather than shrugged at: `-d:icBNodeProf` reports + # `PeekFallback` beside `PeekKind`, and it should stay at zero. + prof pPeekFallback + loadSym(c, sym) + return sym.kindImpl + prof pPeekKind + when defined(icPeekKindCheck): + let peeked = result + loadSym(c, sym) + doAssert peeked == sym.kindImpl, + "peekSymKind disagrees for " & symAsStr & ": peeked " & $peeked & + " but the load says " & $sym.kindImpl + +proc addReexportedEnumFields(c: var DecodeContext; sym: PSym; symAsStr: string; + interf: var TStrTable) = ## When a non-pure enum type is (re-)exported, its fields must also become ## visible (unqualified) to importers. In a from-source build this happens via ## `rawImportSymbol`'s enum handling when the type is imported; the lazy IC ## importer never runs that, so we materialise the fields into the interface ## here, when the export list is processed. + ## + ## Only a TYPE can contribute fields, and almost none of an export list is + ## types — so the kind is read off the def header first (`symKindFast`) rather + ## than by forcing every exported symbol through a full decode to find out. + ## That decode was 290ms of an 8.6s build over 34815 symbols. + if symKindFast(c, sym, symAsStr) != skType: return loadSym(c, sym) if sym.kindImpl != skType or sfPure in sym.flagsImpl: return let et = sym.typImpl @@ -4146,7 +4205,7 @@ proc processTopLevel(c: var DecodeContext; cur: var Cursor; flags: set[LoadFlag] if sym != nil: strTableAdd(interf, sym) icProfStart(tEnumFields) - addReexportedEnumFields(c, sym, interf) + addReexportedEnumFields(c, sym, symAsStr, interf) icProfStop(tEnumFields) skip cur else: diff --git a/compiler/icprof.nim b/compiler/icprof.nim index 8f95594394..5757c688b3 100644 --- a/compiler/icprof.nim +++ b/compiler/icprof.nim @@ -32,7 +32,7 @@ when defined(icBNodeProf): pKind, pTagKindHit, pTagKindMiss, pAstChildren, pSkip, pSon, pLen, pLastSon, pIterYield, pSym, pTyp, pTypTagLit, pOrigin, pNilType, pGenBodyCalls, pInfo, pIfaceExported, pIfaceHidden, pIfaceModules, - pTopNodes, pExportSyms + pTopNodes, pExportSyms, pPeekKind, pPeekFallback, pPeekLoaded TimeSlot* = enum tLoadClosure, tModuleId, tBifLoad, tPosIndex, tTopLevel, tInterfTables, tTransform, tHandOff, tGenBody, tAnalyses,