diff --git a/AGENTS.md b/AGENTS.md index 24ee4ad..b5d3bfc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,4 +116,4 @@ In [docs/decisions.md](docs/decisions.md): - The Swedish ids draw Skatteverket's test series - The US given names come from a mirror of the SSA file - `List` advertises direct descents only -- The path walks are separate loops +- A path draws through its compiled steps diff --git a/docs/decisions.md b/docs/decisions.md index 2aef6aa..e5fc422 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -842,15 +842,14 @@ import drops them by name rather than by a rank a regeneration would move. the set of every descent through a chain of five tables is every subsequence of it, and the direct chain is the one a reader can predict from the tables' parents. -## The path walks are separate loops +## A path draws through its compiled steps -2026-09-28, larv-review on systems-architect's finding; approved 2026-09-28 by lilleman. -Re-tested 2026-09-29 by collapsing them into one loop switching on a mode field. -Goals 2 and 11: `pathCheck`, `pathProbe` and `pathDraw` stay separate loops. -One loop fails `perf_test.go`: the check's leaves and the errors -naming a level leak the walker's contents, and Go tracks a struct's fields as one, so -the draw's pins, arm and memo leak with them. Held by pointer, a repeat of a reference -path rose from 66 to 106 allocations; copied into the walker by value, -`BenchmarkHeld` rose from 5 to 9. A call through an interface or a type parameter leaks -the walker the same way. Valid while Go's escape analysis tracks a struct's fields as -one and treats such a call as leaking its receiver. +2026-09-30, larv-review on the comprehension round the maintainer approved on 2026-09-29; +replaces "The path walks are separate loops". +Goals 2 and 11: `drawSteps` draws every path from its steps, compiled at link by +`pathCheck` or, for a caller's path, by `probePath` into a stack buffer. The check and +the probe stay two loops: one loop leaks the check's leaves and errors with the draw's +state, since Go tracks a struct's fields as one, and a repeat of a reference path rose +from 66 to 106 allocations. `drawSteps` takes its state as parameters and a step holds +no node, so no benchmark gained an allocation. Valid while Go's escape analysis tracks +a struct's fields as one. diff --git a/path.go b/path.go index bff2e7a..b08c7fb 100644 --- a/path.go +++ b/path.go @@ -229,11 +229,6 @@ func (w *pathCheck) run(n node) (node, error) { return leaf, err } -func (w *pathCheck) emit(st pathStep, tail []string) { - st.at = len(w.tail) - len(tail) - w.steps = append(w.steps, st) -} - func (w *pathCheck) walk(n node, tail []string) (node, error) { for descended := false; len(tail) > 0 || descended; { var err error @@ -248,7 +243,7 @@ func (w *pathCheck) walk(n node, tail []string) (node, error) { if r, err = x.route(tail, descended); err != nil { return nil, err } - if err := w.tableSteps(x, r, tail); err != nil { + if w.steps, err = routeSteps(w.steps, &w.pins, x, r, len(w.tail)-len(tail)); err != nil { return nil, err } n, tail, descended = r.next, r.rest, r.descends @@ -264,38 +259,40 @@ func (w *pathCheck) walk(n node, tail []string) (node, error) { if n, err = stepInto(n, tail[0]); err != nil { return nil, err } - w.emit(pathStep{kind: stepField, name: tail[0]}, tail) + w.steps = append(w.steps, pathStep{kind: stepField, at: len(w.tail) - len(tail), name: tail[0]}) tail = tail[1:] } w.leaves = append(w.leaves, n) return n, nil } -func (w *pathCheck) tableSteps(t *table, r tableRoute, tail []string) error { +// routeSteps appends the steps r takes past t, the first at at, pinning in pins the row +// it selects. +func routeSteps(steps []pathStep, pins *pinSet, t *table, r tableRoute, at int) ([]pathStep, error) { if r.sel != "" { - if err := w.pins.selectRow(t, r.sel); err != nil { - return err + if err := pins.selectRow(t, r.sel); err != nil { + return steps, err } - row, _ := w.pins.pinned(t) - w.emit(pathStep{kind: stepSelect, name: r.sel, row: row}, tail) - tail = tail[1:] + row, _ := pins.pinned(t) + steps = append(steps, pathStep{kind: stepSelect, at: at, name: r.sel, row: row}) + at++ } if r.draw { - w.emit(pathStep{kind: stepDraw}, tail) + steps = append(steps, pathStep{kind: stepDraw, at: at}) } switch next := r.next.(type) { case *table: if next != t { - w.emit(pathStep{kind: stepChild, name: next.segment}, tail) + steps = append(steps, pathStep{kind: stepChild, at: at, name: next.segment}) } case *tableRow: - w.emit(pathStep{kind: stepRow}, tail) + steps = append(steps, pathStep{kind: stepRow, at: at}) case *tableColumn: - w.emit(pathStep{kind: stepColumn, name: t.header[next.i]}, tail) + steps = append(steps, pathStep{kind: stepColumn, at: at, name: t.header[next.i]}) default: panic(internalError("%s: a route onto %T", t.segment, r.next)) } - return nil + return steps, nil } // walkEvery walks every variant and keeps the first one's steps: every variant carries @@ -331,13 +328,12 @@ func (w *pathCheck) enter(t *template, rest []string) error { return nil } -// pathProbe proves a path resolves without drawing. Where pathCheck walks every -// variant of a choice, it walks the first, which carriedByAll lets stand for all; at a -// table it selects as pathCheck does. -type pathProbe struct{ pins pinSet } - -func (w *pathProbe) walk(n node, tail []string) (node, error) { - for descended := false; len(tail) > 0 || descended; { +// probePath proves a path resolves without drawing, appending to steps the steps a +// draw takes. Where pathCheck walks every variant of a choice, it walks the first, +// which carriedByAll lets stand for all; at a table it selects as pathCheck does. +func probePath(n node, tail []string, steps []pathStep) ([]pathStep, error) { + var pins pinSet + for full, descended := len(tail), false; len(tail) > 0 || descended; { var err error switch x := n.(type) { case *choice: @@ -351,89 +347,80 @@ func (w *pathProbe) walk(n node, tail []string) (node, error) { if r, err = x.route(tail, descended); err != nil { return nil, err } - if r.sel != "" { - if err := w.pins.selectRow(x, r.sel); err != nil { - return nil, err - } - } - n, tail, descended = r.next, r.rest, r.descends - continue - } - if n, err = stepInto(n, tail[0]); err != nil { - return nil, err - } - tail = tail[1:] - } - return n, nil -} - -// pathDraw draws the rows and variants a proved path reads, pinning the rows in pins; pins is nil -// for a sibling path, which never crosses a table, since a table is only a category. For a memoized -// read, memo keeps the variant drawn at each level of a, so paths sharing a prefix share it. -// Where pathCheck walks every variant of a choice, it walks the one drawn; at a table it selects -// as pathCheck does and draws the row the route asks for. -type pathDraw struct { - s *generatorState - pins *pinSet - memo *drawMemo - a *arm -} - -// drawPath walks w over a path proved first, by a probe or at load, so the walk -// cannot fail. level names n in the panic. -func drawPath(n node, tail []string, level string, w *pathDraw) node { - leaf, err := w.walk(n, tail) - if err != nil { - panic(internalError("%s: %v; the path should have been proved before it was drawn", join(level, joinSegments(tail)), err)) - } - return leaf -} - -func (w *pathDraw) walk(n node, tail []string) (node, error) { - for descended := false; len(tail) > 0 || descended; { - var err error - switch x := n.(type) { - case *choice: - n = w.variant(x, tail) - continue - case *table: - var r tableRoute - if r, err = x.route(tail, descended); err != nil { + if steps, err = routeSteps(steps, &pins, x, r, full-len(tail)); err != nil { return nil, err } - switch { - case r.sel != "": - if err := w.pins.selectRow(x, r.sel); err != nil { - return nil, err - } - case r.draw: - x.drawIn(w.s, w.pins) - } n, tail, descended = r.next, r.rest, r.descends continue } if n, err = stepInto(n, tail[0]); err != nil { return nil, err } + steps = append(steps, pathStep{kind: stepField, at: full - len(tail), name: tail[0]}) tail = tail[1:] } - return n, nil + return steps, nil } -// variant is the variant of c the walk continues into: the one memo keeps for this level -// of the read, drawn once, where the walk has a memo; else one drawn afresh. -func (w *pathDraw) variant(c *choice, rest []string) node { - if w.memo == nil { - return pick(w.s, c) - } - key := w.a.levels[len(w.a.tail)-len(rest)] - n, drew := w.memo.variant[key] - if !drew { - n = resolveChoice(w.s, c) - if w.memo.variant == nil { - w.memo.variant = map[string]node{} +// drawSteps draws the rows and variants a path's steps read from n, pinning the rows in pins; +// pins is nil for a sibling path, which never crosses a table, since a table is only a category. +// For a memoized read, memo keeps the variant drawn at each of levels, so paths sharing a prefix +// share it. +func drawSteps(s *generatorState, n node, steps []pathStep, pins *pinSet, memo *drawMemo, levels []string) node { + for _, st := range steps { + if c, ok := n.(*choice); ok { + n = drawVariant(s, c, memo, levels, st.at) } - w.memo.variant[key] = n + if st.kind == stepField { + var err error + if n, err = stepInto(n, st.name); err != nil { + panic(internalError("%v; a path's steps should have been compiled from the node they are drawn from", err)) + } + continue + } + t, ok := n.(*table) + if !ok { + panic(internalError("a table's step %q on %T", st.name, n)) + } + n = t.drawStep(s, st, pins) + } + return n +} + +func (t *table) drawStep(s *generatorState, st pathStep, pins *pinSet) node { + switch st.kind { + case stepSelect: + if err := pins.pinRow(t, st.row); err != nil { + panic(internalError("%s[%s]: %v", t.segment, st.name, err)) + } + return t + case stepDraw: + t.drawIn(s, pins) + return t + case stepRow: + return t.rowNode + case stepColumn: + return t.formatTemplate.fields[st.name] + case stepChild: + return t.descendant(st.name) + } + panic(internalError("drawStep has no case for step kind %d", st.kind)) +} + +// drawVariant is the variant of c a draw continues into at the step at: the one memo keeps for +// that level, drawn once, where the draw has a memo; else one drawn afresh. +func drawVariant(s *generatorState, c *choice, memo *drawMemo, levels []string, at int) node { + if memo == nil { + return resolveChoice(s, c) + } + level := levels[at] + n, drew := memo.variant[level] + if !drew { + n = resolveChoice(s, c) + if memo.variant == nil { + memo.variant = map[string]node{} + } + memo.variant[level] = n } return n } diff --git a/render.go b/render.go index b5c5da9..3982afd 100644 --- a/render.go +++ b/render.go @@ -40,10 +40,12 @@ func (f *Generator) Fake(path string) (string, error) { // selects or draws. func descend(s *generatorState, root node, segments []string, sc renderScope) (node, error) { // docs/decisions.md#a-path-is-walked-once-without-drawing-before-it-is-walked-for-real - if _, err := (&pathProbe{}).walk(root, segments); err != nil { + var buf [16]pathStep + steps, err := probePath(root, segments, buf[:0]) + if err != nil { return nil, err } - return drawPath(root, segments, "", &pathDraw{s: s, pins: &sc.groupDraws().pins}), nil + return drawSteps(s, root, steps, &sc.groupDraws().pins, nil, nil), nil } // renderOnce renders n as one render, over draws of its own. @@ -188,7 +190,7 @@ func readMemo(s *generatorState, t *template, memo *drawMemo, pins *pinSet, sc r if r, done := memo.value[a.path]; done { return r } - leaf := drawPath(t.head(a.head), a.tail, a.head, &pathDraw{s: s, memo: memo, pins: pins, a: &a}) + leaf := drawSteps(s, t.head(a.head), a.steps, pins, memo, a.levels) if pins != nil { sc = sc.at(leaf, pins) } diff --git a/todo.md b/todo.md index c019b27..eefa87d 100644 --- a/todo.md +++ b/todo.md @@ -40,10 +40,6 @@ maintainer chose on 2026-09-30 to go on with the path steps regardless. This round, in order: -- Draw every path through one interpreter over those steps, `drawSteps`, with `descend` - probing into a stack buffer, deleting `pathDraw`, and replace the decision "The path - walks are separate loops": the prototype held `perf_test.go` once a step held no - node, the probe returned its slice and the interpreter took `levels`. - Fold `checkNoOverlap` and `checkNoRepeatedRead` into `heldCheck`, so `compileFormat` compiles and nothing more. - Fix at compile how `readField` reads an arm, `arm.kind`, deleting its two panics and