akiomik / akiomik/tears

The load harness is declared as a `[[bench]]`, and several open issues trace to that

Aperta
#375 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
documentation
Lingua principale
Rust
Stelle
9
Fork
1
Merge medio
11h 46m
PR unite (30g)
62

Descrizione

## What

`benches/kernel_load.rs` is declared as a `[[bench]]` target and is not a
benchmark in the sense cargo means by that slot.

- It does not use criterion. `benches/gauge.rs` and `benches/kernel_scan.rs`
both `use criterion::…`; `kernel_load.rs` does not.
- It is `harness = false` with a hand-written `fn main() -> ExitCode` that
parses its own arguments — `--smoke`, `--self-test`, `--acceptance`, and
positional row names — and selects every row when given none.

Cargo's `[[bench]]` slot carries a mechanical contract: `cargo bench` runs
every declared bench, and `cargo test --all-targets` runs each with `--test`
for one iteration. This target honours neither, and the contract is what
tooling acts on regardless of what the word means in prose.

## Why it is worth filing now

Four open issues touch the same target, and the reason they exist is not the
same in each, which is worth stating precisely:

- **#368** is the trap directly: the custom `main` ignores `--test`, so
`--all-targets` ran the full scenarios and `just pre-commit` took 22
minutes. Both options that issue weighs — teach the `main` to honour
`--test`, or drop `bench-internals` from the test recipe's feature set —
keep the target where it is.
- **#371** exists because a runner has to name benches one by one. It has to
because "run every declared bench" is not a usable instruction while this
target is among them: it would start an acceptance matrix. With the harness
outside the set, `cargo bench --features …` names nothing and covers
everything, and there is no list to guard.
- **#370**'s defect (no `--features`, so every `required-features` bench is
skipped) is independent of this. What this constrained was the *fix*: the
one-line `cargo bench --features …` was rejected because it would also
reach this target.
- **#374** is downstream of #369, which dropped `--all-targets` because of
this target. The bench coverage and the 22 minutes came from the same
place.

The vocabulary drifted with it. `bench-smoke` is a smoke test, not a
benchmark — RFC 0007 §6 defines it as latency-assertion-free and gating on
completion. `bench-internals` gates three disjoint sets of items, one per
consumer: `LoadObserver` for `gauge`; `RegistryScan` and `CleanupLedgerScan`
for `kernel_scan`; `BenchKernel`, `producer_quit` and `GAUGE_EVENT_FIELDS`
for `kernel_load`, plus the `Command::actions` `producer_quit` is built on.
One feature name covers a bench-only escape and a load-harness escape.

## What is not settled

Filed as a problem statement. Every part of a fix needs investigation first,
and at least these:

1. **Where it would go, which is a profile question before it is a taste
question.** `Cargo.toml` declares no `[profile]` overrides, so the
defaults decide:

| placement | usual invocation | profile | `debug_assertions` |
| --- | --- | --- | --- |
| `benches/` (today) | `cargo bench --bench X` | bench (inherits release) | off |
| `tests/` | `cargo test --test X` | test (inherits dev) | **on** |
| `examples/` | `cargo run --release --example X` | release, named explicitly | off |

RFC 0006 §5's recorded numbers were taken under the bench profile. A
placement whose default invocation is a debug build produces incomparable
numbers and reports them as slowness rather than as failure, so `tests/`
is a trap however well `just test-load` reads.

`examples/` does not settle it either, and the reason is measurable rather
than a matter of taste. `just test` passes `--examples`, and cargo builds
each example as a test harness and runs it. Adding an example declaring
`harness = false` and running `cargo test --examples` executes its `main`:

```
Running unittests examples/zz_probe.rs (target/debug/examples/zz_probe-…)
ZZ_PROBE_MAIN_RAN
```

So an example with a custom `main` is reached by the local test recipe on
every run — the same interaction `--all-targets` had with the bench slot,
arriving through a different flag. It cuts both ways: that is also
compile coverage the gate would keep. The directory's meaning to a reader
("sample code for users") is a separate cost.

A non-published crate is a third shape, with its own publish and
navigation costs — which `docs/testing.md` weighed and rejected for a
different, much smaller subject.

2. **Whether the feature splits.** The item sets are disjoint, so it is
mechanically clean, but it is also independent of the move: any target
outside `src/` sees the public API only and needs a gate wherever it
lives. Against splitting: cargo has no private feature, and
`docs/testing.md` rejects a `test-support` feature on the ground that
`bench-internals` and `loom-core` accept that risk for a minimal surface —
a second door weakens the argument that the exception is minimal. It also
would not separate cleanly: `GAUGE_EVENT_FIELDS` is the gauge's own field
anchor that the harness reads.

3. **What the recipes and features would then be called**, given `bench`,
`bench-test`, `bench-smoke` and `bench-internals` currently name a mix of
both things.

4. **How the contract documents follow.** RFC 0006 §5, RFC 0007 §6, RFC 0014
§13.5 and RFC 0010's ledger name `benches/kernel_load.rs` and `just
bench-smoke` directly. Whether a move is a factual sync or an amendment
is a judgment those documents' own process governs, and it should be
settled before any file moves.

Raised while fixing #370.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.