fix(valgrind): use %p in log file path to avoid multi-process overwrite

[not-matthias]

What?

With --trace-children=yes, every (forked) process wrote to the same valgrind.log file, so the last writer won and earlier processes' logs were lost.

This uses the %p (PID) placeholder in --log-file so each process gets its own valgrind.<pid>.log, matching the existing %p.out (callgrind) and %p.tgtrace (tracegrind) out-file patterns.

Also drops the failure-path read of the now-nonexistent hardcoded valgrind.log.

Closes COD-2747

[codspeed-hq]

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks

---

_{Comparing cod-2747-multi-process-valgrind-logs-can-overwrite-log-file (b1c5547) with main (bd8f33e)}

[greptile-apps]

Greptile Summary

Fixes a multi-process log overwrite bug introduced by --trace-children=yes by replacing the hardcoded valgrind.log path with a %p-keyed pattern, consistent with the existing %p.out and %p.tgtrace out-file conventions. A helper dump_valgrind_logs is added to glob and emit all per-process log files when valgrind exits non-zero.

• log_path changed from valgrind.log to valgrind.%p.log, so each child process writes to its own file instead of overwriting a shared file.
• dump_valgrind_logs iterates the profile folder on failure, filters files matching valgrind.*.log, and emits their contents via debug!, guarded by a log_enabled! check to avoid unnecessary I/O when debug logging is off.

Confidence Score: 5/5

Safe to merge — the change is a targeted, low-risk fix that aligns the log-file naming convention with the existing out-file patterns.

The %p substitution is the established valgrind idiom already used for --callgrind-out-file and --tracegrind-out-file in the same function, so the fix is both correct and idiomatic. The dump_valgrind_logs helper is well-guarded (early return when debug logging is off, silent skip on unreadable entries) and introduces no new failure modes. No regressions were found in the changed path.

No files require special attention.

Important Files Changed

Filename	Overview
src/executor/valgrind/measure.rs	Core fix using %p PID placeholder in --log-file path; adds dump_valgrind_logs helper that safely handles missing/unreadable files and is correctly gated by log_enabled!.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[measure called] --> B[build valgrind cmd\n--log-file=.../valgrind.%p.log]
    B --> C[run_command_with_log_pipe]
    C --> D{valgrind exit\nsuccess?}
    D -- yes --> E{cmd_status == 0?}
    E -- yes --> F[return Ok]
    E -- no --> G[bail: benchmark failed]
    D -- no --> H[dump_valgrind_logs\nprofile_folder]
    H --> I{log::debug\nenabled?}
    I -- no --> J[return early\nno I/O]
    I -- yes --> K[read_dir profile_folder]
    K --> L[for each valgrind.*.log]
    L --> M[debug! file contents]
    M --> N[bail: failed to execute valgrind]

Loading

_{Reviews (5): Last reviewed commit: "fix(valgrind): use %p in log file path t..." | Re-trigger Greptile}

[GuillaumeLagrange]

olgtm