Skip to content

Always append the target PID to profiler output paths - #31

Merged
athei merged 4 commits into
athei:masterfrom
rdbell:profile-pid-path
Sep 14, 2026
Merged

athei merged 4 commits into
athei:masterfrom
rdbell:profile-pid-path

Conversation

@rdbell

@rdbell rdbell commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

When multiple Wine processes inherit the same profiler path, their sidecars can truncate one another's captures. Both X87_PROFILE and X87_SAMPLE now always append .<pid> using the attached target's PID: /tmp/game.prof becomes /tmp/game.prof.12345. The same behavior applies to --sample.

The suffix is simple concatenation, applied before opening or unlinking output. There is no placeholder or extension parsing; %p is literal. Sampling windows use <file>.<pid>.windows. If the target PID is unavailable, that profiler is disabled with a diagnostic before touching files.

README and command-line help document the output names. A bounded integration test, also added to CI, runs two targets concurrently against shared configured paths and checks both profilers, finalized block counters, sampling windows, environment/CLI configuration, literal %p, extensionless names, and preservation of existing files.

Validation on Apple Silicon, macOS 26.5:

  • CMake release build passed.
  • python3 scripts/test_profile_paths.py: all three cases passed, covering six target processes.
  • bash scripts/run_tests.sh --no-build test_x87_loop: 11 passed, 0 failed across the selected configurations.
  • Full regression suite remains configured in PR CI; it was not rerun locally for this follow-up.

Work by Claude Fable 5.1 and GPT-6 Astra.

In cooperative mode Wine re-executes every 32-bit process through the
sidecar, so one game launch is several sidecars: the wineboot helpers,
the process that starts the game, and the game itself, all inheriting
the same X87_PROFILE. The last sidecar to open the path wins, and since
the block profiler appends its counter section only after its target
exits, the game's profile was routinely truncated by a helper's before
it could be finalised. profile_analyze then rejected it as incomplete,
with no way to tell which process the surviving file belonged to.

Expand %p against the pid behind the task port the sidecar received,
for both X87_PROFILE and X87_SAMPLE, before any old output is unlinked
or a new file opened. Without %p the paths behave as before. If the
target pid cannot be resolved the profiler is disabled with a message
rather than writing under the wrong name.

Verified under wine-cx-26.3.0-1 on an M2 Max, macOS 26.5, with a
32-bit Direct3D game: one launch produced x87-block-<pid>.prof for each
of four cooperative sidecars, and the game's file carried a complete
counter section that profile_analyze accepted (2197 blocks).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rdbell pushed a commit to rdbell/x87sidecar that referenced this pull request Sep 5, 2026
…X87_SAMPLE

Our horizonxi-x87 delta only had the X87_SAMPLE half of the %p work (the
narrower expandSamplePid). PR athei#31 (profile-pid-path, filed athei#31)
generalizes it: a shared expandTargetPid(path, targetPid, variable) covering
both X87_PROFILE and X87_SAMPLE, plus the README docs. Adopt PR athei#31's form and
drop the now-orphaned expandSamplePid so development carries all of our upstreamed
%p work, not half of it. Sticky sampler is unchanged.

Verified: cmake build clean; run_tests.sh test_arith 11/11 pass, 0 divergences.
%p output-path expansion is config plumbing with no translation effect.
@athei

athei commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Could we drop %p entirely and always append .<pid> to the provided path for both profilers? /tmp/game.prof would become /tmp/game.prof.12345. With optional %p, users have to discover that their profiles are being silently overwritten before they know how to avoid it. Separate output per process should be the default. This also keeps the code to simple concatenation, with no placeholder or extension parsing.

@athei

athei commented Sep 10, 2026

Copy link
Copy Markdown
Owner

@rdbell Can you incorporate my suggestion?

@rdbell rdbell changed the title Expand %p in X87_PROFILE and X87_SAMPLE to the attached process pid Always append the target PID to profiler output paths Sep 11, 2026
@rdbell

rdbell commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Implemented your suggestion: both profilers now always append .<target-pid> before opening or cleaning up output. Removed %p expansion entirely; there is no extension parsing. Updated the README and help text, and added a bounded concurrent-output regression test to CI.

Local validation passed: release build, six profiler captures across three filename cases, and 11/11 selected x87 loop configurations.

Work by Claude Fable 5.1 and GPT-6 Astra.

@athei athei left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks :)

@athei
athei merged commit 4048fcf into athei:master Sep 14, 2026
1 check passed
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