#35608 contrib: Skip llvm-cov rendering for deterministic fuzz inputs
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35608 contrib: Skip llvm-cov rendering for deterministic fuzz inputs | Waiting on author | 0 (+1) | Strong | 0 + 71 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Optimizes `deterministic-fuzz-coverage` by checking whether raw profile data (`.profdata`) matches byte-for-byte between runs before invoking `llvm-cov show`. Because identical profdata implies identical coverage reports, the expensive report rendering step is skipped for deterministic inputs and only executed if a difference is detected.
Problem: Running `llvm-cov show` for every single fuzz input across two runs is the primary bottleneck of `deterministic-fuzz-coverage`, spending significant developer time rendering identical coverage reports that are immediately discarded.
Reviewability
Paused · Waiting on author
Open objection from maflcko with no author reply for 38 days; review would repeat what has been asked.
Waiting on (from the thread read):
- author: rebase and address suggestion to make parallelism a named argument 2026-09-24
- author: rebase and suggested named parallelism argument 2026-09-24
Agreement
Strong
substantive support, no open objection (jeanpablojp)
Thread notes: Updated rc:3912474146 to answered following maflcko's explanation in rc:3912904082. The PR is waiting on the author to rebase and address maflcko's suggestion on named arguments. / Recorded maflcko's code deduplication suggestion and the author's reply. Marked jeanpablojp's objection answered by maflcko's explanation that dev-only scripts do not need to guard against broken builds. The PR is waiting on the author to rebase and address maflcko's suggestion for a named parallelism argument.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| maflcko (member) | suggestion, maintenance | have check_diff accept the path directly and handle profiling to deduplicate code | answered · author adopted the check_diff deduplication while keeping profdata_eq | answered | yes | 2026-09-01: 'Maybe `check_diff` can just accept the `&Path` and then do everything itself. This would de-dupl a bunch of code.' Status from: 2026-09-03: HowHsu: 'Updated, thanks. Kept the `profdata_eq`, since I feel it is very good for beginners (like me) to understand the code logic.' |
| jeanpablojp (contributor) | objection, correctness | builds missing coverage mapping flags pass determinism check rather than failing fast | answered · maflcko explains that for a dev-only script, extra logic to catch broken builds is not needed | answered | by others | 2026-09-02: 'A build with `-fprofile-instr-generate` but no `-fcoverage-mapping` now prints `Coverage test passed` where master dies with `llvm-cov show failed`' Status from: 2026-09-02: maflcko: 'This is a dev-only script. If there is a "broken build" (because the dev ignored the docs), I think it is fine to not hold their hands with extra logic here.' |
| maflcko (member) | suggestion, interface | make parallelism a proper named argument instead of a positional argument | open | answered | no | 2026-09-24: 'could make it a proper named arg in a new commit' |
Support:
- jeanpablojp (contributor) (Tested ACK, 2026-08-19) [both: addition_overflow, process_message, qa-assets]: Ran both versions on qa-assets with the coverage build left over from my #35809 review with par=4. `addition_overflow` went from 5m49s to 31s and `process_message` from 1h50m to 25m48s, same PASS on both, and the new flow rendered zero reports. Identical `.profdata` would mean an identical report an
- maflcko (member) (ACK, 2026-09-01) [read]: lgtm, left a nit to slim this down a bit [not substantive]
Participants:
- jeanpablojp (contributor, objection, 2 statements 2026-08-19 to 2026-09-02): tested ACKed, but raised a concern about builds missing coverage mapping flags passing determinism
- maflcko (member, support, 4 statements 2026-09-01 to 2026-09-24): ACKed; answered jeanpablojp's concern, noted a rebase is needed, and suggested making parallelism a named argument
Files
107 lines under test/bench/ci.
- contrib/devtools/deterministic-fuzz-coverage/src/main.rs +71/-36
Card
The short text the ranking pass compares across PRs.
This PR optimizes the `deterministic-fuzz-coverage` tool by comparing merged `.profdata` files directly and skipping `llvm-cov show` report rendering when profiles match. This directly addresses excessive runtimes for fuzz determinism audits, reducing test runs on targets like `process_message` from nearly two hours to under 30 minutes. The PR has been tested and approved by multiple contributors with review refactorings adopted.
Data
thread read through 2026-09-24 (7 statements), head c237c9f2ff
labels: Scripts and tools, Needs rebase
Categories
Tools and scripts
P3 · speedup
Substantially accelerates deterministic fuzz coverage checks by bypassing redundant report generation. Review testing noted `addition_overflow` went from 5m49s to 31s and `process_message` from 1h50m to 25m48s when profdata matches.
Membership: Carries the Scripts and tools label and modifies contrib/devtools/deterministic-fuzz-coverage/src/main.rs.
Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 0