Skip to content

feat(valgrind): always enable deterministic RNG - #600

Open
not-matthias wants to merge 3 commits into
mainfrom
cod-4083-enable-deterministic-rng-in-runner
Open

not-matthias wants to merge 3 commits into
mainfrom
cod-4083-enable-deterministic-rng-in-runner

Conversation

@not-matthias

@not-matthias not-matthias commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Always pass --deterministic-rng=yes to valgrind in simulation mode, so AT_RANDOM, getrandom(), /dev/urandom and hardware RNG instructions return fixed values. This removes one source of run-to-run variance.

Blocked on a valgrind-codspeed release: the currently pinned 3.26.0-0codspeed10 does not know --deterministic-rng and would reject it. The pin must be bumped before this merges.

Closes COD-4083


I suggest merging this and releasing a new beta to properly test the feature

@not-matthias not-matthias changed the title feat(valgrind): add experimental flag to enable deterministic RNG feat(valgrind): enable deterministic RNG by default Nov 2, 2026
@not-matthias not-matthias changed the title feat(valgrind): enable deterministic RNG by default feat(valgrind): always enable deterministic RNG Nov 2, 2026
@codspeed

codspeed Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 13.13%

1 warning

⚠️ 7 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

❌ 1 regressed benchmark
✅ 37 untouched benchmarks
⏩ 9 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Walltime memtrack track tar 8.2 s 9.4 s -13.13%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-4083-enable-deterministic-rng-in-runner (93b6701) with main (ea54386)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@not-matthias
not-matthias marked this pull request as ready for review November 2, 2026 08:24
@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] No additional changes are requested beyond the Valgrind release prerequisite already documented by the author.

Summary

Adds --deterministic-rng=yes to every simulation run, covering both Callgrind and Tracegrind.

  • Valgrind simulation runs now ask for fixed random values.

Reviews (1) · Last reviewed commit: "feat(valgrind): always enable determinis..." · Reviewed by Greptile

@GuillaumeLagrange GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I think all thee commits can be squashed though 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants