test(profiling): regression test for #16657#16660
Closed
taegyunkim wants to merge 2 commits into
Closed
Conversation
Codeowners resolved as |
Contributor
|
✨ Fix all issues with BitsAI or with Cursor
|
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Mar 6, 2026
…16657) ## Summary - Split `unwind_greenlets()` into two phases to reduce lock hold time on `greenlet_info_map_lock` - **Phase 1 (under lock):** snapshot greenlet IDs, names, frame pointers, and parent chains into lightweight `GreenletSnapshot` structs - **Phase 2 (lock released):** perform the expensive stack unwinding (`process_vm_readv` / `copy_type`) outside the lock - Add `GreenletSnapshot` struct in `greenlets.h` to hold the snapshotted state ## Motivation Before this fix, `unwind_greenlets()` held `greenlet_info_map_lock` for the entire duration of stack unwinding across **all** tracked greenlets. Since every greenlet switch calls `update_greenlet_frame()` under the same lock, applications with many tracked greenlets (e.g. gunicorn + gevent + psycopg2) experienced severe lock contention — greenlet switches stalled waiting for the sampler, leading to connection pool exhaustion and request timeouts. ### Why asyncio doesn't have this problem Asyncio task unwinding (`unwind_tasks`) doesn't suffer from this because it reads task state directly from CPython internals (linked lists in 3.14+, or `WeakSet` in older versions) via `copy_type()` — no profiler-owned lock is involved during discovery or unwinding. The only profiler lock (`task_link_map_lock`) is held briefly for link-map cleanup, then released before the expensive coroutine stack unwinding begins. Greenlets lack CPython-native tracking, so the profiler must maintain its own `greenlet_info_map` with frame pointers updated on every switch via `update_greenlet_frame()`. This creates a lock shared between the sampler thread (reading) and the application thread (writing on every switch) — the exact contention pattern this fix addresses by making the read-side (Phase 1 snapshot) fast. ### Regression test confirms the issue on main The regression test (`test_gevent_greenlet_switch_not_blocked_by_profiler`) was cherry-picked to main in [#16660](#16660) and fails there, confirming this branch fixes the contention. ## JIRA [SCP-1039](https://datadoghq.atlassian.net/browse/SCP-1039) ## Testing - Added `test_gevent_greenlet_switch_not_blocked_by_profiler` regression test that: - Measures greenlet-switch wall time with 0 vs 2000 idle tracked greenlets (each with 50-deep stacks) while the profiler samples at 5ms intervals - Asserts the ratio stays below 3x to catch lock-contention regressions - Uses `gevent.joinall` with a 30s timeout and `try/finally` cleanup to prevent CI hangs ## Risks - Snapshotted frame pointers may become stale between Phase 1 and Phase 2, but `unwind_frame()` already handles invalid pointers gracefully via `copy_type()` which returns non-zero on failure - No change to public API 🤖 Generated with [Claude Code](https://claude.com/claude-code) [SCP-1039]: https://datadoghq.atlassian.net/browse/SCP-1039?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: taegyun.kim <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Testing
Risks
Additional Notes