perf(internal, profiling): native PeriodicThread avoids awake() after fork()#16545
Conversation
taegyunkim
changed the title
PeriodicThread avoids awake() after fork()
taegyunkim
changed the title
PeriodicThread avoids awake() after fork()PeriodicThread avoids awake() after fork()
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7c6d3f49a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
Performance SLOsComparing candidate taegyunkim/periodic-thread-fork (cb70445) with baseline main (af51ae1) 📈 Performance Regressions (2 suites)📈 iastaspects - 118/118✅ add_aspectTime: ✅ 103.072µs (SLO: <130.000µs 📉 -20.7%) vs baseline: +0.6% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ add_inplace_aspectTime: ✅ 101.738µs (SLO: <130.000µs 📉 -21.7%) vs baseline: -2.3% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.5% ✅ add_inplace_noaspectTime: ✅ 28.320µs (SLO: <40.000µs 📉 -29.2%) vs baseline: +0.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ add_noaspectTime: ✅ 48.894µs (SLO: <70.000µs 📉 -30.2%) vs baseline: -0.7% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ bytearray_aspectTime: ✅ 250.618µs (SLO: <400.000µs 📉 -37.3%) vs baseline: +1.3% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.7% ✅ bytearray_extend_aspectTime: ✅ 637.775µs (SLO: <800.000µs 📉 -20.3%) vs baseline: -3.2% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ bytearray_extend_noaspectTime: ✅ 264.587µs (SLO: <400.000µs 📉 -33.9%) vs baseline: -2.3% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytearray_noaspectTime: ✅ 137.202µs (SLO: <300.000µs 📉 -54.3%) vs baseline: -1.4% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ bytes_aspectTime: ✅ 216.590µs (SLO: <300.000µs 📉 -27.8%) vs baseline: -2.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ bytes_noaspectTime: ✅ 133.123µs (SLO: <200.000µs 📉 -33.4%) vs baseline: -2.1% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ bytesio_aspectTime: ✅ 3.760ms (SLO: <5.000ms 📉 -24.8%) vs baseline: -1.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.0% ✅ bytesio_noaspectTime: ✅ 314.465µs (SLO: <420.000µs 📉 -25.1%) vs baseline: -1.9% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ capitalize_aspectTime: ✅ 89.499µs (SLO: <300.000µs 📉 -70.2%) vs baseline: +0.8% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ capitalize_noaspectTime: ✅ 251.656µs (SLO: <300.000µs 📉 -16.1%) vs baseline: -1.1% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ casefold_aspectTime: ✅ 92.512µs (SLO: <500.000µs 📉 -81.5%) vs baseline: +3.9% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ casefold_noaspectTime: ✅ 308.630µs (SLO: <500.000µs 📉 -38.3%) vs baseline: -1.1% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.1% ✅ decode_aspectTime: ✅ 86.384µs (SLO: <100.000µs 📉 -13.6%) vs baseline: ~same Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.7% ✅ decode_noaspectTime: ✅ 154.188µs (SLO: <210.000µs 📉 -26.6%) vs baseline: -0.2% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.7% ✅ encode_aspectTime: ✅ 84.385µs (SLO: <200.000µs 📉 -57.8%) vs baseline: -0.7% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ encode_noaspectTime: ✅ 140.908µs (SLO: <200.000µs 📉 -29.5%) vs baseline: -0.8% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ format_aspectTime: ✅ 14.605ms (SLO: <19.200ms 📉 -23.9%) vs baseline: -0.2% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.2% ✅ format_map_aspectTime: ✅ 16.385ms (SLO: <21.500ms 📉 -23.8%) vs baseline: -0.3% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +4.5% ✅ format_map_noaspectTime: ✅ 375.296µs (SLO: <500.000µs 📉 -24.9%) vs baseline: +0.3% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ format_noaspectTime: ✅ 300.089µs (SLO: <500.000µs 📉 -40.0%) vs baseline: -2.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ index_aspectTime: ✅ 121.493µs (SLO: <300.000µs 📉 -59.5%) vs baseline: ~same Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ index_noaspectTime: ✅ 40.549µs (SLO: <300.000µs 📉 -86.5%) vs baseline: +1.0% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ join_aspectTime: ✅ 210.950µs (SLO: <300.000µs 📉 -29.7%) vs baseline: -2.2% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.1% ✅ join_noaspectTime: ✅ 141.486µs (SLO: <300.000µs 📉 -52.8%) vs baseline: -1.9% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ ljust_aspectTime: ✅ 493.231µs (SLO: <700.000µs 📉 -29.5%) vs baseline: -3.6% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ ljust_noaspectTime: ✅ 259.715µs (SLO: <300.000µs 📉 -13.4%) vs baseline: -0.5% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ lower_aspectTime: ✅ 294.822µs (SLO: <500.000µs 📉 -41.0%) vs baseline: -2.4% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ lower_noaspectTime: ✅ 237.233µs (SLO: <300.000µs 📉 -20.9%) vs baseline: +0.8% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ lstrip_aspectTime: ✅ 0.269ms (SLO: <3.000ms 📉 -91.0%) vs baseline: -3.1% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.4% ✅ lstrip_noaspectTime: ✅ 0.211ms (SLO: <3.000ms 📉 -93.0%) vs baseline: 📈 +18.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ modulo_aspectTime: ✅ 14.525ms (SLO: <18.750ms 📉 -22.5%) vs baseline: +2.0% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.4% ✅ modulo_aspect_for_bytearray_bytearrayTime: ✅ 14.745ms (SLO: <19.350ms 📉 -23.8%) vs baseline: -1.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ modulo_aspect_for_bytesTime: ✅ 14.365ms (SLO: <18.900ms 📉 -24.0%) vs baseline: -0.5% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.6% ✅ modulo_aspect_for_bytes_bytearrayTime: ✅ 14.624ms (SLO: <19.150ms 📉 -23.6%) vs baseline: ~same Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.3% ✅ modulo_noaspectTime: ✅ 0.360ms (SLO: <3.000ms 📉 -88.0%) vs baseline: -1.0% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ replace_aspectTime: ✅ 18.341ms (SLO: <24.000ms 📉 -23.6%) vs baseline: -0.4% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +4.4% ✅ replace_noaspectTime: ✅ 278.144µs (SLO: <300.000µs -7.3%) vs baseline: -0.9% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.5% ✅ repr_aspectTime: ✅ 310.992µs (SLO: <420.000µs 📉 -26.0%) vs baseline: -2.9% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ repr_noaspectTime: ✅ 47.133µs (SLO: <90.000µs 📉 -47.6%) vs baseline: +1.4% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ rstrip_aspectTime: ✅ 384.359µs (SLO: <500.000µs 📉 -23.1%) vs baseline: -1.5% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ rstrip_noaspectTime: ✅ 183.872µs (SLO: <300.000µs 📉 -38.7%) vs baseline: +0.2% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.3% ✅ slice_aspectTime: ✅ 183.380µs (SLO: <300.000µs 📉 -38.9%) vs baseline: -0.4% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ slice_noaspectTime: ✅ 53.844µs (SLO: <90.000µs 📉 -40.2%) vs baseline: -0.3% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.0% ✅ stringio_aspectTime: ✅ 3.835ms (SLO: <5.000ms 📉 -23.3%) vs baseline: -0.9% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ stringio_noaspectTime: ✅ 381.087µs (SLO: <500.000µs 📉 -23.8%) vs baseline: +7.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.7% ✅ strip_aspectTime: ✅ 269.644µs (SLO: <350.000µs 📉 -23.0%) vs baseline: -2.0% Memory: ✅ 42.821MB (SLO: <46.000MB -6.9%) vs baseline: +4.0% ✅ strip_noaspectTime: ✅ 176.075µs (SLO: <240.000µs 📉 -26.6%) vs baseline: -1.0% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.1% ✅ swapcase_aspectTime: ✅ 335.028µs (SLO: <500.000µs 📉 -33.0%) vs baseline: -1.6% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ swapcase_noaspectTime: ✅ 274.248µs (SLO: <400.000µs 📉 -31.4%) vs baseline: -1.0% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ title_aspectTime: ✅ 320.469µs (SLO: <500.000µs 📉 -35.9%) vs baseline: -2.7% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ title_noaspectTime: ✅ 259.644µs (SLO: <400.000µs 📉 -35.1%) vs baseline: ~same Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ translate_aspectTime: ✅ 489.717µs (SLO: <700.000µs 📉 -30.0%) vs baseline: -2.2% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ translate_noaspectTime: ✅ 422.540µs (SLO: <500.000µs 📉 -15.5%) vs baseline: -2.7% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ upper_aspectTime: ✅ 294.587µs (SLO: <500.000µs 📉 -41.1%) vs baseline: -2.1% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.1% ✅ upper_noaspectTime: ✅ 236.482µs (SLO: <400.000µs 📉 -40.9%) vs baseline: +0.5% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% 📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 510.926µs (SLO: <700.000µs 📉 -27.0%) vs baseline: 📈 +20.0% Memory: ✅ 42.605MB (SLO: <46.000MB -7.4%) vs baseline: +4.4% ✅ ospathbasename_noaspectTime: ✅ 434.442µs (SLO: <700.000µs 📉 -37.9%) vs baseline: ~same Memory: ✅ 42.664MB (SLO: <46.000MB -7.3%) vs baseline: +4.5% ✅ ospathjoin_aspectTime: ✅ 631.297µs (SLO: <700.000µs -9.8%) vs baseline: +1.0% Memory: ✅ 42.507MB (SLO: <46.000MB -7.6%) vs baseline: +3.8% ✅ ospathjoin_noaspectTime: ✅ 633.263µs (SLO: <700.000µs -9.5%) vs baseline: ~same Memory: ✅ 42.507MB (SLO: <46.000MB -7.6%) vs baseline: +3.9% ✅ ospathnormcase_aspectTime: ✅ 348.920µs (SLO: <700.000µs 📉 -50.2%) vs baseline: -0.6% Memory: ✅ 42.526MB (SLO: <46.000MB -7.6%) vs baseline: +4.4% ✅ ospathnormcase_noaspectTime: ✅ 358.302µs (SLO: <700.000µs 📉 -48.8%) vs baseline: +0.7% Memory: ✅ 42.566MB (SLO: <46.000MB -7.5%) vs baseline: +3.7% ✅ ospathsplit_aspectTime: ✅ 488.951µs (SLO: <700.000µs 📉 -30.1%) vs baseline: -0.7% Memory: ✅ 42.684MB (SLO: <46.000MB -7.2%) vs baseline: +4.0% ✅ ospathsplit_noaspectTime: ✅ 500.121µs (SLO: <700.000µs 📉 -28.6%) vs baseline: -0.3% Memory: ✅ 42.625MB (SLO: <46.000MB -7.3%) vs baseline: +4.2% ✅ ospathsplitdrive_aspectTime: ✅ 377.418µs (SLO: <700.000µs 📉 -46.1%) vs baseline: +2.3% Memory: ✅ 42.723MB (SLO: <46.000MB -7.1%) vs baseline: +4.8% ✅ ospathsplitdrive_noaspectTime: ✅ 72.939µs (SLO: <700.000µs 📉 -89.6%) vs baseline: -0.9% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.3% ✅ ospathsplitext_aspectTime: ✅ 458.583µs (SLO: <700.000µs 📉 -34.5%) vs baseline: -0.3% Memory: ✅ 42.644MB (SLO: <46.000MB -7.3%) vs baseline: +4.7% ✅ ospathsplitext_noaspectTime: ✅ 469.920µs (SLO: <700.000µs 📉 -32.9%) vs baseline: +1.1% Memory: ✅ 42.644MB (SLO: <46.000MB -7.3%) vs baseline: +4.6% 🟡 Near SLO Breach (1 suite)🟡 tracer - 6/6✅ largeTime: ✅ 31.539ms (SLO: <32.950ms -4.3%) vs baseline: +0.2% Memory: ✅ 36.707MB (SLO: <39.250MB -6.5%) vs baseline: +4.6% ✅ mediumTime: ✅ 3.117ms (SLO: <3.200ms -2.6%) vs baseline: +0.9% Memory: ✅ 35.527MB (SLO: <38.750MB -8.3%) vs baseline: +5.1% ✅ smallTime: ✅ 362.820µs (SLO: <370.000µs 🟡 -1.9%) vs baseline: +4.5% Memory: ✅ 35.586MB (SLO: <38.750MB -8.2%) vs baseline: +5.7%
|
Codeowners resolved as |
|
@codex review |
taegyunkim
changed the title
PeriodicThread avoids awake() after fork()PeriodicThread avoids awake() after fork()
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bc1d6e814
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
taegyunkim
changed the title
PeriodicThread avoids awake() after fork()PeriodicThread avoids awake() after fork()
|
Given that the problematic commit is not yet released and we're just uploading extra profiles, I'm changing the PR header to perf, dropping release note. |
taegyunkim
added
the
changelog/no-changelog
emmettbutler
left a comment
emmettbutler
left a comment
There was a problem hiding this comment.
looks reasonable to me
Co-authored-by: Brett Langdon <[email protected]>
295a47f
into
main
|
This change is marked for backport to 4.5 and it does not conflict with that branch. |
…er fork() (#16545) ## Description After #14163, we started to see a profiler behavior change in `dd-trace-doe` CI: - `TestLanguage/python/flush/false` expected `0` profiles, got `3`. https://github.com/DataDog/dd-trace-doe/actions/runs/22102391228/job/63875669025 - During pre-fork, `PeriodicThread.stop()` sets the internal request event to wake the periodic threads promptly. - After fork restart, that same request could be interpreted as a real wakeup, causing an immediate `periodic()` run. - For profiling, that means an immediate `upload()` right after fork, which results in unexpected profile exports. What this PR changfes: - Distinguishes a fork-stop wakeup from a real `awake()` request in `PeriodicThread` - Clears the request after fork only when it came from the pre-fork stop path. - Preserves real `awake()` behavior across restart windows. ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers --> Co-authored-by: taegyun.kim <[email protected]> (cherry picked from commit 295a47f)
…er fork() [backport 4.5] (#16900) ## Description Backport of #16545 to 4.5. Co-authored-by: Taegyun Kim <[email protected]>
5 participants
Description
After #14163, we started to see a profiler behavior change in
dd-trace-doeCI:TestLanguage/python/flush/falseexpected0profiles, got3. https://github.com/DataDog/dd-trace-doe/actions/runs/22102391228/job/63875669025During pre-fork,
PeriodicThread.stop()sets the internal request event to wake the periodic threads promptly.After fork restart, that same request could be interpreted as a real wakeup, causing an immediate
periodic()run.For profiling, that means an immediate
upload()right after fork, which results in unexpected profile exports.What this PR changfes:
awake()request inPeriodicThreadawake()behavior across restart windows.Testing
Risks
Additional Notes