IC: correct the module docs to the state the seam is actually in, and measure it

`bnode`'s header still said the generator "has to move together, and it needs
write-side capability this seam does not have". It has moved, and `origin` was
the write-side answer, so the section described a state two commits out of date —
the kind of stale map that sends the next person looking for a problem that is
already solved. Rewritten to say what runs on a cursor now, that `origin` is what
keeps `TLoc.lode` a `PNode`, and that the generator's own in-place rewrites run
on the origin (with the hazard spelled out: where a mutation is read back,
generation has to continue on the origin, because the buffer is a snapshot).

Two blockers were also described wrongly:

* THE ALIAS FAMILY is no longer blocked by field identity — that is exact on a
  bridged buffer. I checked whether `isPartOf` could migrate now and it cannot,
  for a different reason: every call site passes `d.lode` as one operand and that
  is a `PNode`, so a generic `isPartOf` would still be handed a `PNode` on one
  side and buy nothing. It moves when `TLoc.lode` does.
* `sym`'s note said `isPartOf` "CANNOT be migrated as written" without
  qualifying that this is a FILE-path property; a bridged buffer hands back the
  object it was given, and the grinder asserts exactly that.

And the cost, which I flagged twice as unmeasured and is now measured on a
50-module target:

    baseline (PNode)            6.75s
    bridge built, not read      6.79s   -- encoding is inside the noise
    generator driven off it     8.85s   -- +31%

So the encoder is as cheap as claimed and the whole cost is in READING: `son` is
O(i), `kind` indexes a memo per call, `sym`/`typ` go through the nav, `origin` is
a hash lookup per location built. None of it is inherent and none of it has been
optimised. Since a compile is mostly frontend, codegen itself is slowed by well
over 31%.

Verified: both configurations build; cursor-driven and `PNode`-driven output
still identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
This commit is contained in:
Araq
2026-08-30 16:25:13 +02:00
parent 603953376d
commit 8afd306b0d
2 changed files with 65 additions and 30 deletions

View File

@@ -161,34 +161,50 @@
## `genericHead` and the `kids` / `ikids` / `paramTypes` / `signature`
## iterators, which name the child and go through `[]`.
##
## WHERE THE LEAF MIGRATION ENDS. Of the 191 `PNode`-taking procs across
## `cgen` and the `ccg*` files, 160 EMIT — they take a `Builder`/`TLoc`
## out-param or write into `p` directly. Those do not move one at a time: they
## are one mutual recursion rooted at `expr`/`genStmts`, so the whole generator
## has to move together, and it needs write-side capability this seam does not
## have. What is left over splits into named blockers rather than a backlog, and
## each is recorded at its own site:
## WHAT RUNS ON A CURSOR, AND WHAT STILL DOES NOT. `expr` and the ~160 emitters
## under it read the routine body through a cursor: `cgen.genProcBody` is handed
## `BNode(bodyBuf.rootCursor)` and the whole generator follows. It had to land in
## one step — `expr` dispatches to all of them — and what made that possible
## without changing `TLoc` was `nodebridge`'s ORIGIN TRACKING: a cursor can name
## the `PNode` it was encoded from, so `TLoc.lode` stays a `PNode` holding the
## same object a tree-driven build would have stored, and the identity
## comparisons already in the backend keep meaning what they meant.
##
## * A type's RECORD TREE is not a body. `asgnComplexity`, `isEmptyCaseObjectBranch`,
## `containsOpaqueImportcFieldAux`, `genRecordFieldsAux`, `fillResult` and the
## type-section walkers read `PType.n`, which stays a `PNode` by design (see
## the `BType` note below). They are not migration candidates at all.
## `origin` is also how the generator's own REWRITES survive. It does mutate in
## places (`mAppendSeqElem`, `mNewSeq`, `genSetLengthSeq`, `genWasMoved`,
## `genArrToSeq` replace a child or a type in place; `genEnumToStr`, `mAsgn` and
## `spawn` build fresh trees), and those run on the origin. Where the mutation is
## then read back, generation continues on the origin too — the buffer does not
## see the write, so a cursor would keep reading the slot as encoded. That is the
## one hazard to remember when migrating anything else that writes.
##
## What is left is not a backlog, it is named blockers, each recorded at its own
## site:
##
## * A type's RECORD TREE is not a body. `asgnComplexity`,
## `isEmptyCaseObjectBranch`, `containsOpaqueImportcFieldAux`,
## `genRecordFieldsAux`, `fillResult` and the type-section walkers read
## `PType.n`, which stays a `PNode` by design (see the `BType` note below).
## They are not migration candidates at all.
## * RETURNS A NODE OR NIL — `ccgutils.getPragmaStmt`. `.bif` spells a missing
## child as a `DotToken` INSIDE a tree; there is no nil token to hand back as a
## return value and a `Cursor` is not nilable. The fix is to split the
## predicate out, as `stmtsContainPragma` does.
## * WRITES TO THE NODE — `cgen.easyResultAsgn` does `incl n.flags, nfPreventCg`.
## The seam is read-only and a `Cursor` points into a shared token buffer.
## * NEEDS RENDERING — `ccgcalls.preventNrvo` interpolates `$le` into
## `warnObservableStores`. Reconstructing source text is a different job from
## reading a node, and only a diagnostic wants it.
## * NEEDS STABLE FIELD IDENTITY — `lhsDoesAlias` and `potentialAlias` through
## `aliases.isPartOf`, which compares field `sym.id`. See the note on `sym`
## below: it is not idempotent for fields, so this one is not blocked on
## effort, it is blocked on a property the seam does not currently have.
## predicate out, as `stmtsContainPragma` does. The same reason keeps the
## assignment DESTINATION a `PNode` throughout the call family (`genCall`
## passes nil), along with `check`, `exvar`, `stepNode` and a try's `fin`.
## * WRITES TO THE NODE — `cgen.easyResultAsgn` sets `nfPreventCg`. Unlike the
## generator's rewrites it cannot use `origin`, because it runs BEFORE the
## handoff and the buffer is a snapshot taken after it.
## * THE ALIAS FAMILY stays on `PNode`, and NOT for the reason first recorded
## here. Field identity is exact on a bridged buffer (see `sym` below), so that
## is no longer what blocks it — but every call site passes `d.lode` as one
## operand, and that is a `PNode`, so a generic `isPartOf` would still be
## handed a `PNode` on one side and buy nothing. It moves when `TLoc.lode`
## does, not before.
## * MIXED REPRESENTATION — `potentialAlias` and `getPotentialReads` build and
## consume a `seq[PNode]` alongside the node, so both sides would have to be
## the same spelling.
## the same spelling. `genParams` materialises its arguments as origins for
## exactly this reason.
##
## There is deliberately no `BType` alongside `BNode`. Types stay `PType`s even
## under `newIcBackend` — `typ` below returns one — because the backend asks
@@ -482,14 +498,20 @@ when defined(newIcBackend):
## grinder asserts that for the non-field case at every node.
##
## The consequence is not theoretical. A proc that reads a field sym twice
## and compares IDENTITY is correct on a `PNode` and wrong on a `Cursor`:
## `aliases.isPartOf` does exactly that (`a[1].sym.id != b[1].sym.id`, to
## decide whether two accessor chains touch the same field) and so CANNOT be
## migrated as written. What codegen actually consumes for a field is the
## and compares IDENTITY is correct on a `PNode` and wrong on a FILE-BACKED
## cursor: `aliases.isPartOf` does exactly that (`a[1].sym.id != b[1].sym.id`,
## to decide whether two accessor chains touch the same field).
##
## A BRIDGED buffer does not have this problem — `nodebridge` hands back the
## object it was given, and the grinder asserts idempotence for fields there
## while excluding them here. So field identity is no longer what keeps the
## alias family on `PNode`; see the note in the module header for what does.
## What codegen actually consumes for a field is the
## name it re-navigates the reclist with (`lookupFieldAgain`) plus, for
## tuples, the position — which is also the tolerance the grinder applies —
## so the fix is either to compare fields that way or to give a field token
## a stable identity. The latter needs the token's own position as a key,
## so the fix for the FILE path is either to compare fields that way or to
## give a field token a stable identity. The latter needs the token's own
## position as a key,
## and `nifcore.Cursor` keeps that pointer private, so it is not something
## this module can do alone.
result = symAt(currentNav()[], n.raw)

View File

@@ -41,8 +41,21 @@
## `aliases.isPartOf` from moving to the seam (see `bnode.sym`). A bridged
## buffer hands back the same object every time, so code that compares field
## identity is correct on it.
## * It is cheap. No string formatting, no pool lookups for names, no index
## seeks — the encoder is a tree walk and two `seq.add`s.
## * The ENCODER is cheap, and that part is measured: no string formatting, no
## pool lookups for names, no index seeks, just a tree walk and two `seq.add`s.
## Building a buffer for every routine and NOT reading it costs 6.79s against a
## 6.75s baseline on a 50-module target — inside the noise.
##
## READING is not free, and that is where the cost of the whole seam sits.
## Driving the generator off cursors takes the same target from 6.75s to 8.85s,
## **+31%**, stable across interleaved runs. Since a compile is mostly frontend,
## codegen itself is slowed by considerably more than 31%. The suspects are the
## per-access costs a `PNode` does not have: `son(n, i)` is O(i) because it skips
## from the first child, `kind` checks the tag pool and indexes a memo on every
## call, `sym`/`typ` go through the nav, and `origin` is a hash lookup on every
## location built. None of that is inherent — `son` could cache, `origin` could
## key on something cheaper — but none of it has been optimised, and the number
## is here so nobody has to rediscover it before deciding whether to.
##
## WHAT IT IS NOT. The buffer is transient and process-local: `(bsym …)` means
## nothing without the tables beside it, so a bridged buffer must never be