Skip to content

perf(profiling): consolidate greenlet switch updates - #19655

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
mainfrom
taegyunkim/profiling-greenlet-switch-update
Aug 13, 2026
Merged

perf(profiling): consolidate greenlet switch updates#19655
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
mainfrom
taegyunkim/profiling-greenlet-switch-update

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

Consolidates the two native frame updates performed for each greenlet switch into one private record_greenlet_switch() call and one greenlet_info_map mutex acquisition.

The existing behavior is preserved: the origin frame is always updated, while the running target frame is left unchanged when it is needed for parent-greenlet stack unwinding.

This is an independently mergeable prerequisite extracted from #18724.

Testing

  • scripts/lint fmt -- ddtrace/profiling/_gevent.py tests/profiling/test_gevent.py tests/profiling/collector/test_stack.py
  • scripts/lint cformat
  • scripts/lint profiling-native-check
  • scripts/lint checks
  • git diff --check
  • Fresh CPython 3.12 native extension build
  • Native linked-parent switch regression covering preserved and updated target frames
  • Existing high-cardinality greenlet switch contention regression

Benchmark

The benchmark isolates the native operation changed by this PR. Each measured run performs 1,000,000 logical greenlet switch updates after 10,000 warm-up updates:

  • baseline e11d640110: two update_greenlet_frame() native calls and two mutex acquisitions per update
  • candidate 93702898f8: one record_greenlet_switch() native call and one mutex acquisition per update

Both _stack extensions were built in Release mode from the exact commits, then run in the same container pinned to CPU 0 with --cpuset-cpus=0.

Results over five runs:

  • before: 0.531681 seconds median, runs [0.528339, 0.531681, 0.532128, 0.534566, 0.529234]
  • after: 0.366206 seconds median, runs [0.380415, 0.368066, 0.360803, 0.362898, 0.366206]
  • change: approximately 31% faster for the isolated native update

Environment:

  • AWS KVM VM, Intel Xeon Platinum 8175M at 2.50 GHz
  • 16 vCPUs, 8 cores with 2 threads per core, 61 GiB RAM
  • Linux 6.8.0-1055-aws, x86-64
  • Docker server 29.5.2, container pinned to one CPU
  • CPython 3.12.13, GCC 14.2.0, glibc 2.41
  • container image sha256:ce7c46dbb7f07d352aecd756e19fd7f39550a72d4a13e68d30f02f5448eebd69
Benchmark script
import statistics
import time

from ddtrace.internal.datadog.profiling import stack


ITERATIONS = 1_000_000
RUNS = 5
ORIGIN_ID = 101
TARGET_ID = 102

stack.track_greenlet(ORIGIN_ID, "origin", False)
stack.track_greenlet(TARGET_ID, "target", False)

if hasattr(stack, "record_greenlet_switch"):

    def update():
        stack.record_greenlet_switch(ORIGIN_ID, False, TARGET_ID, None, True)

else:

    def update():
        stack.update_greenlet_frame(ORIGIN_ID, False)
        stack.update_greenlet_frame(TARGET_ID, None)


def run_once():
    start = time.perf_counter()
    for _ in range(ITERATIONS):
        update()
    return time.perf_counter() - start


for _ in range(10_000):
    update()
runs = [run_once() for _ in range(RUNS)]

stack.untrack_greenlet(ORIGIN_ID)
stack.untrack_greenlet(TARGET_ID)

print(f"median={statistics.median(runs):.6f}s runs={runs}")

This is an isolated native-call benchmark, not an end-to-end application throughput claim. The earlier 67% figure was discarded after reproducing with exact baseline and candidate builds because the candidate run had not activated the profiling hook.

Risks

Low. The changed Python and native APIs are private and ship together. Existing parent-greenlet frame retention remains unchanged.

Additional Notes

The GIL release around native state mutation is retained intentionally. It was introduced by #14852 in commit 54a3a0ea35 to prevent potential thread-pool deadlocks while waiting for profiler mutexes. This change reduces two such release windows per greenlet switch to one.

No release note is needed because this is an internal performance optimization with no user-facing API or behavior change.

@taegyunkim taegyunkim added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 12, 2026
@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codeowners resolved as

Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.

No remaining files require a CODEOWNERS review.

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 12, 2026

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

There are 6 circular imports that already exist on the base branch and have not been changed by this PR.

Show existing cycles (showing 5 of 6 shortest)
ddtrace.contrib.internal.django.patch -> ddtrace.contrib.internal.django.response -> ddtrace.contrib.internal.django.patch
ddtrace.contrib.internal.pytorch._distributed -> ddtrace.contrib.internal.pytorch._rank_root -> ddtrace.contrib.internal.pytorch._distributed
ddtrace.appsec._common_module_patches -> ddtrace.appsec._listeners -> ddtrace.appsec._common_module_patches
ddtrace.errortracking._handled_exceptions.bytecode_injector -> ddtrace.errortracking._handled_exceptions.callbacks -> ddtrace.errortracking._handled_exceptions.collector -> ddtrace.errortracking._handled_exceptions.bytecode_reporting -> ddtrace.errortracking._handled_exceptions.bytecode_injector
ddtrace.llmobs -> ddtrace.llmobs._evaluators -> ddtrace.llmobs._evaluators.format -> ddtrace.llmobs._experiment -> ddtrace.llmobs

To see all cycles, download the cycles-base.json and cycles-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/cycles.py compare cycles-base.json cycles-pr.json

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 12, 2026

Copy link
Copy Markdown

Dependency direction analysis

⚠️ Existing dependency direction violations

There are 256 dependency direction violations that already exist on the base branch and have not been changed by this PR.

Show existing violations (showing 5 of 256 highest severity)
ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=134)
ddtrace.internal.opentelemetry.context -×-> ddtrace.trace  (product:opentelemetry -> product:tracing, score=132)
ddtrace.debugging._signal.model -×-> ddtrace.trace  (product:debugging -> product:tracing, score=132)
ddtrace.llmobs._utils -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=132)
ddtrace.llmobs._integrations.vllm -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=132)

To see all violations, download the layers-base.json and layers-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/layers.py compare layers-base.json layers-pr.json

@datadog-official

datadog-official Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 9370289 | Docs | Datadog PR Page | Give us feedback!

@taegyunkim taegyunkim added the Profiling Continous Profling label Aug 12, 2026
@taegyunkim
taegyunkim requested a balanced review from Copilot August 12, 2026 18:52
@taegyunkim

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: c33b4f2f9d

ℹ️ 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".

Copilot AI 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.

Pull request overview

Consolidates greenlet switch frame updates into one native call and mutex acquisition to reduce profiling overhead.

Changes:

  • Adds combined Python/native greenlet-switch update APIs.
  • Preserves conditional parent-frame retention.
  • Adds forwarding coverage and updates related documentation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ddtrace/profiling/_gevent.py Consolidates switch updates.
stack.cpp Exposes the native switch API.
sampler.cpp Updates both frames under one lock.
sampler.hpp Declares the combined method.
threads.cc Updates lock-contention commentary.
_stack.pyi Updates native API typing.
__init__.pyi Updates exported API typing.
test_gevent.py Adds forwarding regression coverage.
test_stack.py Updates contention-test documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ddtrace/internal/datadog/profiling/stack/src/stack.cpp Outdated
Comment thread tests/profiling/test_gevent.py Outdated
@taegyunkim
taegyunkim requested a balanced review from Copilot August 12, 2026 20:08
@taegyunkim

Copy link
Copy Markdown
Contributor Author

@codex review

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 4f3ad005a9

ℹ️ 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
taegyunkim marked this pull request as ready for review August 12, 2026 22:07
@taegyunkim
taegyunkim requested a review from a team as a code owner August 12, 2026 22:07

@KowalskiThomas KowalskiThomas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very cool find and change.

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit df0dc65 into main Aug 13, 2026
480 of 481 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the taegyunkim/profiling-greenlet-switch-update branch August 13, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR. Profiling Continous Profling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants