Closed sanchda closed 3 days ago
Benchmark execution time: 2024-10-31 20:22:14
Comparing candidate commit 2bfef1b in PR branch sanchda/crashtracking_safety
with baseline commit d92f667 in branch main
.
Found 1 performance improvements and 1 performance regressions! Performance is the same for 49 metrics, 2 unstable metrics.
execution_time
[+28.395ns; +36.942ns] or [+2.409%; +3.134%]execution_time
[-81.858ns; -72.228ns] or [-2.994%; -2.642%]Omitted due to size.
How does this interact with the PHP sidecar?
How does this interact with the PHP sidecar?
It should not. In a future PR I think many of us hope to harmonize the two implementations, but there are a few questions about how packaging would work in that universe. Right now I think the priority is to fix the outstanding defects, provide a sane assessment of risk for the runtimes which will rely on this implementation of crashtracking, and then harmonize over a more relaxed timeline.
Curiously, I see here that vfork availability isn't what I thought it was on macos. Will check that out after I finish writing some documents and tests.
Attention: Patch coverage is 0%
with 932 lines
in your changes missing coverage. Please review.
Project coverage is 70.56%. Comparing base (
d92f667
) to head (2bfef1b
).
:wave: adding @bwoebi and @kevingosse as subject matter experts. I'd really appreciate a review of the systems interactions in crash_handler.rs
to make sure we're doing what we said we would on the tin.
What does this PR do?
Changes the way the crashtracker receiver works
This PR isn't yet complete, as it requires more tests and an RFC. But please feel free to comment on what you see!
Motivation
Some runtimes actually call
waitpid(-1,...
anticipating an ECHILD, which is incompatible with the idea of a long-lived receiver process. We also wanted to improve some of the safety around how signals work.Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.
APMLP-286