Skip to content

arm64: clear pooled assembler nodes when a compile returns them - #617

Open
cyclistmass wants to merge 1 commit into
Clozure:arm64from
cyclistmass:arm64-asm-pool-retention
Open

arm64: clear pooled assembler nodes when a compile returns them#617
cyclistmass wants to merge 1 commit into
Clozure:arm64from
cyclistmass:arm64-asm-pool-retention

Conversation

@cyclistmass

Copy link
Copy Markdown
Contributor

The instruction and label pools live for the whole compile, and
return-dll-nodes parks nodes with their slots intact. A parked instruction
keeps SOURCE, TEMPLATE and PARSED-OPERANDS. A parked label keeps REFS,
and on the vinsn path a vinsn-label also keeps NAME. The pools therefore hold the finished
compile's whole IR graph until reuse clears those slots. That graph survives
the ephemeral generations and tenures, at about 5.4 KB per compile.

Cost over one ANSI suite run:

arm64 before arm64 after x86-64
full GCs 29 6 5
generation-2 GCs 166 35
GC time 4.11 s 2.52 s

Total collections are 4913 in both halves. The same collections happen, and
fewer of them promote. A 20,000-call COMPILE loop on the same image adds 3
full and 15 generation-2 collections before the patch, and 1 and 2 after.

I re-measured at your dd80005e, with this patch as the only change:
stage-11 CPU 49.73 s to 47.84 s.

The patch clears the reference-carrying slots in an unwind that runs before
each section returns to the pool. The walk handles both node types, because
labels splice into the same section as instructions and return to
*instruction-freelist*. Numeric slots keep their values, and reuse resets
them.

The patch adds reset-instruction-elements, and the two arm64 call sites that
own a pooled section wrap their body so it runs before the section returns.

The change stays inside compiler/ARM64/. A clear hook in return-dll-nodes
would cover every backend, and I have measured only arm64, so I have left the
others alone. Two things I did check. Nothing in the tree ever returns a node to
*x86-lap-label-freelist* -- there is a defvar and an allocation site and no
third use -- so an x86 lap label is never parked in the first place. x86-64 does
pool frags through *frag-freelist*, and make-frag clears them on reuse
rather than on return, which is the same shape as this bug; whether it retains
anything is a separate measurement I have not made.


Verified at this base. Built and run on linuxarm64 at ec578745 with all
ten patches applied: ANSI 21679 tests, 0 failures. tests/ccl.lsp 243 tests, 0
failures. Image 281a49e5, kernel 67bb66b4.

The instruction and label pools are permanent roots, and
return-dll-nodes parks nodes with their slots intact.  A parked
instruction keeps SOURCE and PARSED-OPERANDS, and a parked label keeps
REFS and, in the vinsn path, a vinsn-label NAME.  Until reuse clears
them, the pools hold the finished compile's whole IR graph, which
survives the ephemeral generations and tenures.  About 5.4 KB tenures
per compile (measured at d61da8d).

One ANSI suite run makes 29 full GCs on arm64 against 5 for x86-64 CCL
1.12.2 under the same GC configuration.  A loop of 20,000 trivial COMPILE calls adds 3 full GCs
on arm64 against 0 on x86-64.  x86-64 pools only frags and lap labels,
and those carry no operand graph.

Clear the reference-carrying slots in an unwind that runs before each
section returns to the pool.  The walk handles both node types,
because labels splice into the same section as instructions and return
to *instruction-freelist*.  The numeric slots keep their values, and
reuse resets them.  A per-pool clear hook in return-dll-nodes is the
more general fix, but this change stays inside compiler/ARM64/ so the
other ports carry no risk.

Signed-off-by: Mauro DiBenedetto <maurodibenedetto@gmail.com>
@xrme

xrme commented Aug 27, 2026

Copy link
Copy Markdown
Member

I wonder if freelisting the instructions and labels is even worth it. It could be better to cons away, and the egc deal with it.

How hard would it be for you to run that test, i.e., have make-instruction always return a freshly-allocated instruction instance?

@cyclistmass

Copy link
Copy Markdown
Contributor Author

I ran it, and it points your way: with the pool gone the compiler is faster than
it is with this patch.

First, the label half generalizes. I told you above that nothing in the tree
ever returns a node to *x86-lap-label-freelist*. That is not an x86 quirk —
it holds for every label pool in the tree, including arm64's own
*label-freelist* and *lap-label-freelist*. return-dll-nodes has exactly one
caller, with-dll-node-freelist, and every one of its call sites names an
instruction, vinsn or frag pool. No call site anywhere names a label pool. So
labels are already always freshly consed, everywhere, and half of what you are
asking about is not in service.

While confirming that: arm64's %make-label returns name on its reuse branch,
where the ones in risc-lap.lisp and x86-lap.lisp return lab. Its caller
stores that in *labels*. Unreachable today for exactly the reason above, but
wrong if that pool were ever fed. Worth a one-line fix whichever way this goes.

Second, the instruction pool measures worse than it looks. Instrumenting
make-instruction over a compile-file of lib/sequences.lisp: 64,584 calls,
77.9% got a usable instruction, 22.1% got a label — which fails the typep, is
discarded, and a fresh instruction is consed anyway — and 0% found the pool
empty. So better than one pop in five is pure overhead.

The A/B. One image, one process, arms swapped by redefinition, 8 pairs with
the order alternated, a full GC before each arm untimed and a full GC after each
arm timed, so the arm that conses more pays for its own cleanup. linuxarm64,
1.13 (v1.13-424-gec578745).

Worth being explicit about the baseline: arm A is this branch, so it already
has this patch in it.
I confirmed reset-instruction-elements is fbound in
the image I measured on. The comparison is pool-plus-this-patch against no pool.

compile-file lib/sequences.lisp, median of 8:

total consed GC
pool 318 ms 21.99 MB 43 ms
always cons 304 ms 23.30 MB 34 ms

4.4% faster, 5.9% more consing, and less GC time — which is your point about
the egc. A pooled node is long-lived and tenures, and a tenured object holding
references costs more than a nursery object that dies immediately.

A generated 120-form lambda, 12 compiles per rep, same protocol: 999 → 987 ms,
1.2% faster, 7.1% more consing. Repeat runs land the same way every time.

Both arms emit the same code. I checked that against a noise floor rather than
byte-identity, because two compiles of one file under one arm already differ by
more bytes (611) than the two arms differ from each other (416).

Two things bias the measurement against the winning arm, so treat 4.4% as a
floor: the cons arm runs a closure wrapping %make-instruction where the pool
arm runs the original compiled function, and it still runs this patch's
reset-instruction-elements — clearing slots on nodes it then throws away.

So I think this PR is the wrong fix. It makes pooling cheaper; not pooling is
cheaper still, and it deletes code rather than adding it. If the pool goes, the
retention this patch clears cannot happen at all.

I wrote that patch rather than just offering it. It is on a separate branch,
cyclistmass:arm64-remove-asm-instruction-pool, one commit on top of
ec578745 — I did not touch this PR's branch, so this one stays as you last
read it.

Both freelist bindings go, both pool-reading wrappers go, and the two struct
constructors take the names their callers already use, so the wrappers disappear
instead of becoming pass-throughs. The two sections that owned a pooled list now
own a plain header. Six lines added, thirty-eight removed, all inside
compiler/ARM64/. It removes the %make-label fault above by deletion rather
than by fixing it.

Built and run on linuxarm64 at that base: ANSI 21679 tests, 0 failures;
tests/ccl.lsp 243 tests, 0 failures. Image 274823a4, kernel 67bb66b4.

Throw it away if you would rather keep the pool — it costs you nothing to
ignore. If you want it, I will close this PR in its favor. I have measured only
arm64; the same shape is in the other backends and I have not run them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants