Skip to content

chore: small fixes for simulation subprocess tracking - #513

Merged
not-matthias merged 2 commits into
mainfrom
chore/small-fixes
Aug 21, 2026
Merged

chore: small fixes for simulation subprocess tracking#513
not-matthias merged 2 commits into
mainfrom
chore/small-fixes

Conversation

@not-matthias

@not-matthias not-matthias commented Aug 21, 2026

Copy link
Copy Markdown
Member

No description provided.

ArgAction::Set made the value mandatory, so the bare flag failed with
"a value is required". Presence-only matches the false default.
Per-thread dumps are now enabled with --simulation-track-subprocess
instead of --cycle-estimation, so thread separation follows the option
that changes what is measured rather than how cycles are estimated.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes subprocess tracking a presence-only CLI flag and ties both Valgrind instrumentation startup and thread separation to that setting.

  • Allows --simulation-track-subprocess without a value.
  • Enables inherited instrumentation and per-thread Callgrind output only while subprocess tracking is enabled.
  • Leaves cycle estimation responsible only for --cycle-estimation=yes.

Confidence Score: 5/5

The PR appears safe to merge based on the accepted review findings, with the explicitly acknowledged experimental Tracegrind limitation remaining a deliberate follow-up decision.

No unacknowledged, concretely reachable failure was established in the changed CLI parsing or Callgrind output behavior.

Important Files Changed

Filename Overview
src/cli/shared.rs Simplifies subprocess-tracking argument parsing to support the bare boolean flag; no actionable current compatibility failure was established.
src/executor/valgrind/measure.rs Rebinds thread separation to subprocess tracking; the known Tracegrind incompatibility is explicitly acknowledged in the PR description.

Reviews (1): Last reviewed commit: "refactor(valgrind): tie --separate-threa..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Aug 21, 2026

Copy link
Copy Markdown

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.

✅ 17 untouched benchmarks


Comparing chore/small-fixes (ab1a9fb) with main (e5587f4)

Open in CodSpeed

@not-matthias
not-matthias merged commit 1dcc738 into main Aug 21, 2026
23 checks passed
@not-matthias
not-matthias deleted the chore/small-fixes branch August 21, 2026 10:04
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