IC: build a module's hidden interface on demand, not on every load

`interfHidden` was 1.05s of a cold Atlas build: 1.70M hidden-symbol stubs against 0.29M exported ones, created by every `nim m` for every module it imports. The table is reached ONLY through `modulegraphs.interfSelect` with `optImportHidden`, and that flag is set in exactly one place — an `import x {.all.}`. Almost nothing reads it. So it is built when something asks. Every read goes through `interfSelect`, so guarding its four call sites is complete; `modulegraphs` already imports `ast2nif`, so the call is direct. The reason the first attempt at this failed, recorded because it is not guessable: **a module has two FileIndexes.** `registerNifSuffix` keys `filenameToIndexTbl` by the NIF SUFFIX and mints a `fikNifModule` entry, while the graph indexes `g.ifaces` by the module's `fikSource` file, and `DecodeContext.mods` is keyed by the former. Asking it with the latter misses every time, silently — `import x {.all.}` then reported "undeclared identifier" for a symbol that was right there. The lazy builder therefore takes a SUFFIX. Two more conditions are load-bearing: clear the pending flag only when the build SUCCEEDS, since an import whose `.s.bif` the build has not produced yet must be retried rather than written off for the rest of the process; and build into a LOCAL table before assigning it back, since loading symbols can grow `g.ifaces` and leave a `var` alias into it dangling. Atlas, 204 modules: frontend 9.98s -> 8.96s loading 4.57s -> 3.55s InterfTables 1161ms -> 79ms hidden stubs 1.70M -> 0 cold serial 21.59s -> 19.79s cold parallel 11.11s -> 10.47s Also fixes a scanner gap the test exposed. `import x {.all.}` serialises as `(pragmax x (pragmas all))`, which `deps.nim.parseImportPath` did not recognise, so it fell into the unknown-subtree skip and the import was DROPPED from the static graph — the build only learned about it from the `.s.deps` sidecar a round later, after a round that failed with "requires precompiled NIF for import". Correct, but a wasted round and an alarming error line for an ordinary import. `tests/ic/timporthidden.nim` covers it: `{.all.}` sees the private symbols, and the sibling case (a plain `import`) still rejects them under both `--ic:on` and `--ic:off`. ic 42/42, `koch boot -d:release` equal executables.
2026-09-02 11:53:45 +00:00 · 2026-08-31 19:38:07 +02:00
parent f48efa4b1f
commit 55b244efff
6 changed files with 119 additions and 53 deletions
--- a/compiler/ast2nif.nim
+++ b/compiler/ast2nif.nim
@@ -3698,23 +3698,10 @@ proc populateInterfaceTablesFromIndex(c: var DecodeContext; module: FileIndex;
  # (moduleId can add to c.mods which would invalidate Table iterators)
  var indexTab = move c.mods[module].index

-  # Add all symbols to interf (exported interface) and interfHidden
-  # A BACKEND stage cannot read the hidden-only half, so it does not build it.
-  # `interfHidden` is reached exclusively through `modulegraphs.interfSelect`,
-  # which picks it only when `optImportHidden` is in the module's options — and
-  # that flag is set in exactly one place, `importer.importModuleAs`, i.e.
-  # during sem. `cg`/`emit` build their module symbols in `moduleFromNifFile`
-  # and never import anything, so the flag cannot be set and the table cannot be
-  # selected. The saving is the bulk of this proc: on a 68-module target the
-  # hidden branch stubs 2.0M symbols against the exported branch's 0.33M.
-  #
-  # Exported symbols are still added to BOTH tables. That costs little beside
-  # the above and leaves `interfHidden` a coherent view (a module with no hidden
-  # symbols) rather than an empty one, should anything ever consult it.
-  let conf = c.infos.config
-  let backendStage = conf.cmd == cmdNifC and
-                     (conf.icBackendStage == "cg" or conf.icBackendStage == "emit")
-
+  # Only the EXPORTED half; `buildHiddenInterface` below does the rest, on
+  # demand. Exported symbols go into both tables, which costs little and leaves
+  # `interfHidden` a coherent view of a module with no hidden symbols rather
+  # than an empty one.
  prof pIfaceModules
  for nifName, entry in indexTab:
    if entry.vis == Exported:
@@ -3723,17 +3710,48 @@ proc populateInterfaceTablesFromIndex(c: var DecodeContext; module: FileIndex;
      if sym != nil:
        strTableAdd(interf, sym)
        strTableAdd(interfHidden, sym)
-    elif not backendStage and not nifName.startsWith("`t"):
-      prof pIfaceHidden
-      # do not load types, they are not part of an interface but an implementation detail!
-      #echo "LOADING SYM ", nifName, " ", entry.offset
-      let sym = loadSymFromIndexEntry(c, module, nifName, entry, thisModule)
-      if sym != nil:
-        strTableAdd(interfHidden, sym)

  # Move index table back
  c.mods[module].index = move indexTab

+proc buildHiddenInterface*(c: var DecodeContext; suffix: string;
+                           interfHidden: var TStrTable): bool {.discardable.} =
+  ## The hidden-only half of a loaded module's interface, materialised on
+  ## demand. Deferred because almost nothing reads it: `interfHidden` is reached
+  ## exclusively through `modulegraphs.interfSelect`, which picks it only when
+  ## `optImportHidden` is in the module's options, and that flag is set in
+  ## exactly one place — an `import x {.all.}`. Building it eagerly was 1.05s of
+  ## a cold Atlas build: 1.70M hidden stubs against 0.29M exported ones, made by
+  ## every `nim m` for every module it imports and read by none of them.
+  ##
+  ## Takes the module SUFFIX, not a FileIndex, and that is the whole trick. A
+  ## module has TWO FileIndexes: `registerNifSuffix` keys
+  ## `filenameToIndexTbl` by the suffix string and mints a `fikNifModule` entry,
+  ## while the graph indexes `g.ifaces` by the module's `fikSource` file. `c.mods`
+  ## is keyed by the former. Asking it with the latter misses every single time,
+  ## silently, and an `import x {.all.}` then reports "undeclared identifier"
+  ## for a symbol that is right there.
+  ##
+  ## Returns false when the artifact is not on disk yet — an import the build
+  ## has not produced. The caller must leave the request PENDING then: writing
+  ## it off on that first miss costs the module its hidden symbols for the rest
+  ## of the process.
+  let conf = c.infos.config
+  if not fileExists((getNimcacheDir(conf) / RelativeFile(suffix & ".s.bif")).string):
+    return false
+  let module = moduleId(c, suffix, {})
+  if not c.mods.hasKey(module): return false
+  var indexTab = move c.mods[module].index
+  for nifName, entry in indexTab:
+    if entry.vis != Exported and not nifName.startsWith("`t"):
+      prof pIfaceHidden
+      # do not load types, they are not part of an interface but an implementation detail!
+      let sym = loadSymFromIndexEntry(c, module, nifName, entry, suffix)
+      if sym != nil:
+        strTableAdd(interfHidden, sym)
+  c.mods[module].index = move indexTab
+  result = true
+
 proc moduleSymbolStubs*(c: var DecodeContext; module: FileIndex): seq[PSym] =
  ## Stubs for every non-type symbol serialized in `module`'s NIF index. The
  ## per-module backend uses this to emit the routines a module OWNS: procs are
--- a/compiler/bnode.nim
+++ b/compiler/bnode.nim
@@ -149,35 +149,20 @@
 ##   up front — a macro-generated `{.all.}` import cannot be seen syntactically,
 ##   and guessing wrong loses symbols silently.
 ##
-##   That lazy conversion has been ATTEMPTED and does not work as a
-##   substitution. The shape is easy: `modulegraphs` already imports `ast2nif`
-##   (the dependency runs that way, so no callback hook is needed), every read of
-##   `interfHidden` goes through `interfSelect`, and its 4 call sites all have
-##   the graph and the module to hand. It builds and it keeps privacy — a plain
-##   `import` still rejects a private symbol under both `--ic:on` and
-##   `--ic:off` — but `import x {.all.}` then fails under `--ic:on` with
-##   "undeclared identifier", where it works today and works under `--ic:off`.
-##   The failure is silent and it is entangled with the driver's discovery loop:
-##   the module is not in `DecodeContext.mods` at the point the lookup asks, on
-##   a COLD cache and on a warm one.
-##
-##   Three fixes were tried and none of them is it, recorded so nobody
-##   re-guesses them:
-##     * clearing `hiddenPending` only when the build SUCCEEDS, so an import
-##       whose `.s.bif` does not exist yet is retried rather than written off.
-##       Necessary — one early miss otherwise costs the module its hidden
-##       symbols for the whole process — but not sufficient.
-##     * loading the module inside the lazy builder (`moduleId` is idempotent)
-##       instead of assuming the caller already did.
-##     * building into a LOCAL `TStrTable` and assigning it back, in case the
-##       `var` alias into `g.ifaces` was being invalidated by the seq growing
-##       during the load.
-##   After all three, `ensureHiddenIface` still reports the module missing from
-##   `DecodeContext.mods` on every one of its 19 calls. Whatever populates
-##   `hiddenPending` and whatever populates `c.mods` are not agreeing about
-##   which module they mean, and that is where the next attempt should start —
-##   with `moduleId`'s FileIndex against `moduleFromNifFile`'s, instrumented at
-##   both ends, before any more of the mechanism is written.
+##   That conversion is DONE (`modulegraphs.ensureHiddenIface` +
+##   `ast2nif.buildHiddenInterface`), and the thing that made it hard is worth
+##   knowing: a module has TWO FileIndexes. `registerNifSuffix` keys
+##   `filenameToIndexTbl` by the NIF SUFFIX and mints a `fikNifModule` entry,
+##   while the graph indexes `g.ifaces` by the module's `fikSource` file.
+##   `DecodeContext.mods` is keyed by the former. Asking it with the latter
+##   misses every single time, silently, and `import x {.all.}` then reports
+##   "undeclared identifier" for a symbol that is right there. So the lazy
+##   builder takes a SUFFIX, not a FileIndex. Two further conditions are also
+##   load-bearing: clear the pending flag only when the build SUCCEEDS (an
+##   import whose `.s.bif` does not exist yet must be retried, not written off),
+##   and build into a LOCAL table before assigning it back (loading symbols can
+##   grow `g.ifaces`, which would leave a `var` alias into it dangling).
+##   `tests/ic/timporthidden.nim` is what says all of this still holds.
 ## * `-d:icBridgeOnly` builds the buffer but generates off the tree, which
 ##   separates the ENCODER's cost from the READER's. Encoding is free — it does
 ##   not show in wall time at all.
--- a/compiler/deps.nim
+++ b/compiler/deps.nim
@@ -522,6 +522,18 @@ proc parseImportPath(s: var Stream; t: var PackedToken): seq[string] =
      for r in parseImportPath(s, t):
        result.add op & r
      if t.kind == ParRi: t = next(s)  # skip closing ')'
+    elif tag == "pragmax":
+      # `import x {.all.}` serialises as `(pragmax x (pragmas all))`. Without
+      # this it fell into the unknown-subtree skip below and the import was
+      # DROPPED from the static graph: the build only learned about it from the
+      # `.s.deps` sidecar a round later, after a round that failed with
+      # "requires precompiled NIF for import". Correct, but a wasted round and
+      # an alarming error line for an ordinary import.
+      t = next(s)                      # skip 'pragmax' tag
+      result = parseImportPath(s, t)   # the path is the first child
+      while t.kind != ParRi and t.kind != EofToken:
+        discard parseImportPath(s, t)  # the pragma list; consumed, not a path
+      if t.kind == ParRi: t = next(s)  # skip closing ')'
    elif tag == "bracket":
      t = next(s)                      # skip 'bracket' tag
      while t.kind != ParRi and t.kind != EofToken:
--- a/compiler/modulegraphs.nim
+++ b/compiler/modulegraphs.nim
@@ -36,6 +36,10 @@ type
    pureEnums*: seq[PSym]
    interf: TStrTable
    interfHidden: TStrTable
+    hiddenPending: bool ## `interfHidden` holds only the exported half so far;
+                        ## `ensureHiddenIface` materialises the hidden-only
+                        ## symbols on first use. See
+                        ## `ast2nif.buildHiddenInterface`.
    uniqueName*: Rope

  Operators* = object
@@ -257,6 +261,25 @@ proc toBase64a(s: cstring, len: int): string =
    result.add cb64[a shr 2]
    result.add cb64[(a and 3) shl 4]

+proc ensureHiddenIface(g: ModuleGraph; pos: int) =
+  ## Materialise a loaded module's hidden-only interface the first time anything
+  ## asks for it. Every READ of `interfHidden` goes through `interfSelect`, so
+  ## guarding those sites is complete.
+  if g.ifaces[pos].hiddenPending:
+    when not defined(nimKochBootstrap):
+      # By SUFFIX: `c.mods` and `g.ifaces` use different FileIndexes for the
+      # same module (see `buildHiddenInterface`). Into a LOCAL table, because
+      # loading symbols can grow `g.ifaces` and a `var` alias into it would then
+      # point at the freed buffer. Cleared only on success, so an import whose
+      # `.s.bif` does not exist yet is retried rather than written off.
+      var tab = g.ifaces[pos].interfHidden
+      if buildHiddenInterface(ast.program,
+                              cachedModuleSuffix(g.config, FileIndex pos), tab):
+        g.ifaces[pos].interfHidden = tab
+        g.ifaces[pos].hiddenPending = false
+    else:
+      g.ifaces[pos].hiddenPending = false
+
 template interfSelect(iface: Iface, importHidden: bool): TStrTable =
  var ret = iface.interf.addr # without intermediate ptr, it creates a copy and compiler becomes 15x slower!
  if importHidden: ret = iface.interfHidden.addr
@@ -292,6 +315,7 @@ proc initModuleIter*(mi: var ModuleIter; g: ModuleGraph; m: PSym; name: PIdent):
  assert m.kind == skModule
  mi.modIndex = m.position
  mi.importHidden = optImportHidden in m.options
+  if mi.importHidden: ensureHiddenIface(g, mi.modIndex)
  result = initIdentIter(mi.ti, g.ifaces[mi.modIndex].interfSelect(mi.importHidden), name)

 proc nextModuleIter*(mi: var ModuleIter; g: ModuleGraph): PSym =
@@ -299,6 +323,7 @@ proc nextModuleIter*(mi: var ModuleIter; g: ModuleGraph): PSym =

 iterator allSyms*(g: ModuleGraph; m: PSym): PSym =
  let importHidden = optImportHidden in m.options
+  if importHidden: ensureHiddenIface(g, m.position)
  for s in g.ifaces[m.position].interfSelect(importHidden).data:
    if s != nil:
      yield s
@@ -334,10 +359,12 @@ proc reexportedLocalSyms*(g: ModuleGraph; m: PSym): seq[ItemId] =

 proc someSym*(g: ModuleGraph; m: PSym; name: PIdent): PSym =
  let importHidden = optImportHidden in m.options
+  if importHidden: ensureHiddenIface(g, m.position)
  result = strTableGet(g.ifaces[m.position].interfSelect(importHidden), name)

 proc someSymAmb*(g: ModuleGraph; m: PSym; name: PIdent; amb: var bool): PSym =
  let importHidden = optImportHidden in m.options
+  if importHidden: ensureHiddenIface(g, m.position)
  var ti: TIdentIter = default(TIdentIter)
  result = initIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden), name)
  if result != nil and nextIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden)) != nil:
@@ -1122,6 +1149,7 @@ when not defined(nimKochBootstrap):
          strTableAdd(interf, inner)
      g.ifaces[fIdx.int].interf = interf
      g.ifaces[fIdx.int].interfHidden = interfHidden
+      g.ifaces[fIdx.int].hiddenPending = true

  proc moduleFromNifFile*(g: ModuleGraph; fileIdx: FileIndex;
                          flags: set[LoadFlag] = {}): PrecompiledModule =
@@ -1167,6 +1195,8 @@ when not defined(nimKochBootstrap):
    result = loadNifModule(ast.program, fileIdx,
                           g.ifaces[fileIdx.int].interf,
                           g.ifaces[fileIdx.int].interfHidden, flags)
+    # The hidden-only half was not built; `ensureHiddenIface` will, if asked.
+    g.ifaces[fileIdx.int].hiddenPending = true
    result.module = m
    # Restore the module symbol's persisted flags (see ast2nif `(modflags)`);
    # `cgen.genTopLevelStmt` gates the destructor pass on `sfInjectDestructors`.
--- a/tests/ic/mimporthidden.nim
+++ b/tests/ic/mimporthidden.nim
@@ -0,0 +1,4 @@
+proc pub*(x: int): int = x + 1
+
+proc secret(): int = 7      # no `*`
+proc hiddenToo(x: int): int = x
--- a/tests/ic/timporthidden.nim
+++ b/tests/ic/timporthidden.nim
@@ -0,0 +1,17 @@
+discard """
+output: '''42'''
+"""
+
+# `import x {.all.}` makes x's PRIVATE symbols visible. Under IC that means the
+# hidden half of a loaded module's interface has to be there — and it is now
+# built on demand rather than at load time, because almost nothing ever reads it
+# (1.70M hidden stubs against 0.29M exported ones on a cold Atlas build).
+#
+# The trap the first attempt fell into: a module has TWO FileIndexes. `c.mods`
+# in the decode context is keyed by the one `registerNifSuffix` mints for the
+# NIF suffix; `g.ifaces` is indexed by the module's source file. Asking one with
+# the other misses silently, and this test is what says so.
+
+import mimporthidden {.all.}
+
+echo secret() + hiddenToo(35)