Skip to content

fix(zram): a same-process ratio is load-immune but NOT instrumentation-immune - #3290

Closed
noahgift wants to merge 1 commit into
mainfrom
PMAT-3237-zram-timing
Closed

noahgift wants to merge 1 commit into
mainfrom
PMAT-3237-zram-timing

Conversation

@noahgift

@noahgift noahgift commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Coverage Nightly has been red every night. #3227 fixed the test it was named for (a lock in
apr_serve_tests.rs) and landed 02:19Z — and the 08:18Z run on a1a19c4fc, which contains
that fix
, failed on a different one:

thread 'benchmark::tests::test_decompression_faster_than_compression'
  panicked at crates/aprender-zram-core/src/benchmark.rs:522:9
test result: FAILED. 403 passed; 1 failed

Why the obvious defence didn't work

This test is already a same-process ratio — the shape its neighbour
test_f058_entropy_overhead_minimal was rewritten into, whose comment explains why:

load-immune because both measurements are taken in the same process, microseconds apart,
under whatever load exists

Load-immune is not instrumentation-immune, and they are different axes. cargo llvm-cov's
overhead scales with the number of instrumented branches executed, and the compress and
decompress paths do not execute the same number. Under coverage this ratio measures
instrumentation density, not the algorithm — and no amount of same-process-ness helps, because
the distortion isn't noise, it's systematic and asymmetric.

The ratio had already been widened once for exactly this reason — "In debug builds with
coverage instrumentation, decompression may be slower"
, 0.25 — and the lane failed on it
anyway. Widening again is the treadmill f058's own comment names: "the number it measures is
how loaded the machine is."

So: assert where the number means something, report where it doesn't

#[cfg(coverage)]      print both rates; assert only that BOTH paths produced a
                      rate — true under instrumentation, and a zero is still caught
#[cfg(not(coverage))] the 0.25 ratio, unchanged

cargo llvm-cov compiles with --cfg=coverage — verified, not assumed:

$ cargo llvm-cov show-env
__CARGO_LLVM_COV_RUSTC_WRAPPER_RUSTFLAGS='-Cinstrument-coverage--cfg=coverage'

A compile-time split, not a runtime guess at whether we're instrumented.

Proven, because a passing test doesn't say which branch ran

without the cfg      0 "informational" lines  -> the ASSERT branch ran
with --cfg=coverage  1 "informational" line   -> the REPORT branch ran
mutation 0.25 -> 1000.0, no cfg               -> FAILED

The third row is the one that matters: the assertion is still load-bearing where it's kept,
so this narrows the claim rather than deleting the test.

The distinction is the finding

Three patterns now live in this file, and they are not interchangeable:

defends against
f058's ratio rewrite load
F053's report-don't-assert an absolute bound being meaningless in a required check
this a ratio being meaningless only where the instrument distorts it

fmt clean, clippy -D warnings 0 errors, roadmap additive (reserialised=0).

Closes #3185

Coverage Nightly had two independent failures and #3185 documented BOTH: the
GPU-adapter pair (test_silu_backward_at_zero / …_length_mismatch, fixed by the
#3116 class) and this one. #3185 was closed COMPLETED on 2026-09-14T03:01Z with the
second still live, and the lane failed again 5 h 17 m later on exactly the test its
body names. Reopened with the evidence; this PR is what closes it.

An earlier revision of this body said "a red-nightly fix with no filed issue" — that
was a triage miss, not a fact.

ont-delta: none — a test-assertion fix; it adds no entity, shape, verdict reason or resolves
target of its own.

🤖 Generated with Claude Code

…n-immune

Coverage Nightly has been red every night. #3227 fixed the test it was named for
(a lock in apr_serve_tests.rs) and landed 02:19Z; the 08:18Z run on a1a19c4 —
which CONTAINS that fix — failed on a different one:

    thread 'benchmark::tests::test_decompression_faster_than_compression'
      panicked at crates/aprender-zram-core/src/benchmark.rs:522:9
    test result: FAILED. 403 passed; 1 failed

WHY THE OBVIOUS DEFENCE DID NOT WORK. This test is ALREADY a same-process ratio,
which is the shape its neighbour test_f058_entropy_overhead_minimal was rewritten
into and which that comment explains: "load-immune because both measurements are
taken in the same process, microseconds apart, under whatever load exists".

Load-immune is not instrumentation-immune, and they are different axes.
`cargo llvm-cov`'s overhead scales with the number of instrumented branches
EXECUTED, and the compress and decompress paths do not execute the same number.
So under coverage this ratio measures instrumentation density, not the algorithm —
no amount of same-process-ness helps, because the distortion is not noise, it is
systematic and asymmetric.

The ratio had already been widened once for exactly this reason ("In debug builds
with coverage instrumentation, decompression may be slower", 0.25) and the lane
failed on it anyway. Widening again is the treadmill f058's own comment names:
"the number it measures is how loaded the machine is."

SO: assert where the number means something, report where it does not.

    #[cfg(coverage)]      print the two rates; assert only that BOTH paths
                          produced a rate, which stays true under instrumentation
                          and still catches a zero
    #[cfg(not(coverage))] the 0.25 ratio, unchanged

`cargo llvm-cov` compiles with `--cfg=coverage` — verified, not assumed:

    $ cargo llvm-cov show-env
    __CARGO_LLVM_COV_RUSTC_WRAPPER_RUSTFLAGS='-Cinstrument-coverage--cfg=coverage'

so this is a compile-time split, not a runtime guess at whether we are instrumented.

PROVEN, because a passing test does not say which branch ran:

    without the cfg      0 "informational" lines  -> the ASSERT branch ran
    with --cfg=coverage  1 "informational" line   -> the REPORT branch ran
    mutation 0.25 -> 1000.0, no cfg               -> FAILED

The third row is the one that matters: the assertion is still load-bearing where
it is kept, so this narrows the claim rather than deleting the test.

This is the third pattern in this file, beside f058's ratio rewrite and F053's
report-don't-assert, and the distinction between them is the finding: f058 defends
against LOAD, F053 gives up on an ABSOLUTE bound, and this one gives up on a RATIO
only where the instrument distorts it.

fmt clean, clippy -D warnings 0 errors, roadmap additive (reserialised=0).

ont-delta: none — a test-assertion fix; it adds no entity, shape, verdict reason or
resolves target of its own.

Pmat-Ticket: PMAT-3237
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

§13.11 rung 1 — quorum shadow verdict

S13-SHADOW pr=3290 head=dd9948a2b083412c267ab121e45c593ef10c701c verdict=REFUSE class=Q1 arm_rc=1

Shadow mode: this records a verdict and merges nothing. A refusal
to arm is not a block (§13 adds zero rows to §7) — the pull request is
exactly as green as it was.

@noahgift

Copy link
Copy Markdown
Contributor Author

Superseded by #3295, which carries this change unmodified along with eight other
collision-free CI/guard PRs as a single merge-queue entry.

Not abandoned and not rejected: the commits are merged into
PMAT-3231-batch-1-ci-guards verbatim (clean=9 conflicted=0), and this branch is
retained — reopening is one click if #3295 does not land.

Why: a successful merge_group CI run is median 78 min with max_entries_to_merge = 1,
so nine entries is ~12 h of queue for 623 insertions. As one entry it is 78 minutes.
Root cause and the full five-whys: #3294.

@noahgift noahgift closed this Sep 15, 2026
@noahgift noahgift mentioned this pull request Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant