Fix three CLOS guards whose read-time class literals never match (eql-gf memoization, static slot-value, validate-superclass) - #1812
Conversation
Any generic function with an eql specializer took a full dispatch miss on every call: compute-applicable-methods, compute-effective-method and a fresh effective-method-function, every time. Measured on this tree (3.0.1-73-g2cf5bb5e4, boehm and boehmprecise identically): 2-method eql gf 6784 B/call, call history stays empty clasp-ffi:%mem-ref 19448 B/call, 321 us/call, call history empty class-specialized gf 0 B/call clasp-ffi:%mem-ref / %mem-set define one eql method per foreign type, so every CFFI foreign memory access paid this. After the fix both drop to 0 B/call and 137 ns, and the call history fills normally (27 entries for %mem-ref). Root cause is the guard in miss-info deciding whether an eql-specialized call may be memoized. It compared (class-of generic-function) against a read-time literal, #.(find-class 'standard-generic-function). In the built image that comparison is always false, so miss-info fell through to the (t nil) clause and never called memoize-eql-specialized. Nothing was ever added to the call history, so the next call missed again. The literal is not stale. In the running image the module literal, the live class and (class-of gf) are all eq to one another; the bytecode decodes correctly (jump-if-8 +12 lands exactly on the called-fdefinition of memoize-eql-specialized); and instrumenting outcome shows every input to the cond is correct, with ok nil and final-methods of length one. Yet witness counters on memoize-eql-specialized, specializers-combinate and call-history-find-key all stay at zero. Why the comparison evaluates false in cross-clasp-compiled code is not explained; recompiling the identical source at runtime makes it work. Filed separately. Replacing the read-time literal with a runtime find-class fixes it. The lookup is on the miss path only, which after this change is taken once per key rather than once per call. static-gfs::uncustomizable-slot-p carried the same read-time-literal guard and was dead the same way, silently disabling the static slot-value/slot-boundp optimization for every standard class: it returned NIL for a plain standard-class with a standard-effective-slot-definition even though both eq tests are true evaluated at runtime. This also explains why toggling clos::*optimize-slot-value* moved allocation by <0.1% -- the optimization was already off. Adds a regression test that fails before and passes after: an eql-specialized gf must leave a non-empty call history.
validate-superclass permits a standard-class and a funcallable-standard-class
to appear in each other's superclass chain, via two clauses guarded by
read-time class literals. Both evaluate false in the built image, so the mixed
case was rejected outright:
(defclass plain-sc () ())
(defclass fsc-from-sc (plain-sc) ()
(:metaclass clos:funcallable-standard-class))
;; => Class #<STANDARD-CLASS PLAIN-SC> is not a valid superclass for
;; #<FUNCALLABLE-STANDARD-CLASS FSC-FROM-SC>
Called directly, both directions returned NIL where T is required.
Same root cause as the eql-specializer memoization guard in this branch: a
#.(find-class ...) literal that is eq to the live class yet compares false in
cross-clasp-compiled code. Unlike that one, this is a correctness bug rather
than a lost optimization -- Clasp refuses hierarchies AMOP permits.
The lookups are deferred behind the (eq c1 c2) fast path and use errorp nil, so
the common same-metaclass case does no lookup at all, and bootstrap cannot trip
over a metaclass that is not yet registered. Verified by a clean Lisp bootstrap
of boehmprecise -- kernel and both images regenerated from source, no errors,
1979 successes with zero unexpected failures.
Adds two MOP tests: both directions of validate-superclass, and the defclass
that previously failed.
dg1sbg
changed the title
|
Note on the red Evidence it is flaky rather than caused by this PR:
Local verification of this branch, macOS arm64:
I have re-triggered CI on this PR. |
|
Second CI flake on this branch, a different one, same conclusion. The re-triggered run cleared the earlier Same commit
No commit in between — the second run was a close/reopen re-trigger of the identical SHA. So across three runs of this branch, three different jobs failed for three unrelated reasons (weak-hash GC test, runner shutdown, one ANSI numeric test), while every other job passed each time. Nothing points at this change, which touches only CLOS class-identity guards. The four unexpected successes in the same ANSI run suggest that expected-failure list has drifted on macOS as well. Local state for this branch remains: boehm 1977, boehmprecise 1979, zero unexpected failures, including a full from-scratch rebuild. |
|
Cross-platform validation, since this PR's CI has been red twice for reasons unrelated to it (see the two comments above — a weak-hash GC flake and an ANSI flake, each disproven by identical-commit contradiction). Built and tested on a second platform to remove any doubt that the change is macOS/arm64-specific:
Same totals on both, across two architectures, two LLVM major versions, and two build modes. The Linux configuration is deliberately the same one whose CI job went red ( Tested here as part of an integration branch merging six open PRs together, so this also confirms these three guards coexist with the shmem, slot-value, FFI and hardening work rather than only passing in isolation. The six tests added by this PR pass on both platforms: Root cause of the underlying compiler defect is now fully diagnosed in #1811 — it is not |
|
Merging #1815 first is what makes this PR's CI able to hold still. The two red jobs here are not going to clear on their own, and re-triggering is close to a coin flip. Across three runs of this branch, three different jobs failed for three unrelated reasons, while every other job passed each time:
Every one of those is disproven as a regression by identical-commit contradiction — the same SHA
Suggested order:
Merging this one first would work too, but its CI would stay unreliable and every future re-trigger keeps rolling the same dice. There is nothing to fix in this PR's code: it passes 1979 successes with zero unexpected failures on macOS arm64 / LLVM 22 / native and x86-64 Linux / LLVM 18 / bytecode, the latter across 13 consecutive full suite runs in the same configuration whose CI job goes red. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Summary of my Linux verification of this PR. This supersedes my earlier comments in this thread (now collapsed) — one of them reported a regression that turned out not to reproduce, and I withdraw it. Both fixes verified, with a baselinex86-64 Linux, LLVM 18,
The Regression suite: clean1968 successes / Withdrawn: my earlier "drops 122 tests, split the PR" reportI earlier reported 1846 successes in bytecode mode and attributed it to I chased three candidate explanations for the anomalous 1846 and none survived: So the original 1846 is unreproduced and unexplained. I am not going to offer a fourth theory. Nothing here should block this PR. Two unrelated observations,
|
This comment has been minimized.
This comment has been minimized.
1 participant
Fixes #1811.
Three guards in CLOS compare a class against a read-time literal,
#.(find-class ...). All three evaluate false in the built image, so all three silently take their fallback path. Two cost performance; the third is a correctness bug.1.
miss.lisp:266— eql-specialized generic functions never memoizeAny generic function with an
eqlspecializer takes a full dispatch miss on every call. Its call history stays permanently empty, socompute-applicable-methods,compute-effective-methodand a fresheffective-method-functionrun every single time. Class-specialized generic functions are unaffected.False in the built image, so
miss-infofalls through to(t nil)and never callsmemoize-eql-specialized. Nothing is added to the call history, so the next call misses again.This is not discriminating-function recomputation:
updatedpis always NIL, soforce-discriminatornever runs. The raw miss path is simply taken every time — which is why the cost is flat across the first and last key rather than looking like a search.clasp-ffi:%mem-refclasp-ffi:%mem-set%mem-refcall historyclasp-ffi:%mem-ref/%mem-setdefine oneeqlmethod per foreign type, so every CFFI foreign memory access was paying this. Cost scaled at roughly 6300 B + 288 B/method. Confirmed one layer up: compiled(cffi:mem-ref p :float off)goes from that to 0.00 B/call.2.
svuc.lisp:21-23— static slot-value optimization disableduncustomizable-slot-preturned NIL for a plainstandard-classwith astandard-effective-slot-definition, silently disabling the staticslot-value/slot-boundpoptimization for every standard class. Botheqtests are true when evaluated at runtime.3.
class.lisp:174-177—validate-superclassrejects AMOP-legal hierarchiesThis one is a correctness bug. The two clauses permitting a
standard-classand afuncallable-standard-classin each other's superclass chain are both dead, so the mixed case is refused:Called directly, both directions returned NIL where T is required.
Fix
Runtime
find-classin all three. Invalidate-superclassthe lookups are deferred behind the existing(eq c1 c2)fast path and useerrorp nil, so the common same-metaclass case does no lookup and bootstrap cannot trip over a metaclass that is not yet registered.cl:find-classis a C++CL_DEFUNhash lookup, not a generic function, so it is safe on the dispatch miss path.Verification
Six new tests, in
fastgf.lispandmop.lisp. Each fails before and passes after.boehm: 1974 successes, zero unexpected failures.boehmprecise: 1976, zero unexpected failures.boehmprecise: kernel Lisp and both images deleted and regenerated from source — no errors, 1979 successes, zero unexpected failures. This specifically checks that replacing load-time literals with runtime lookups does not destabilise bootstrap ordering, since all three sites run during bootstrap (validate-superclasson everydefclass,miss-infoon every dispatch miss,uncustomizable-slot-pat every kernel compile).Worth a second look
The change fixes the symptom; the reason the comparisons fail is not explained, which is why #1811 is open separately. The module literal, the live class and
(class-of x)are alleqto one another; the bytecode decodes correctly; every input to thecondis correct — yet witness counters on the guarded functions stay at zero, and recompiling the identical source at runtime works.Not all such guards fail:
class.lisp:184,class.lisp:307andgeneric.lisp:284behave correctly. The audit in #1811 rules out per-class, per-file,defunvsdefmethod, test-vs-value, andclass-ofvssi::instance-classas the discriminator. Whatever the real cause is, it can silently disable any future#.(find-class ...)guard the same way — which is exactly how the slot-value optimization sat dead without anyone noticing.