Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix profiling in Python 3.10 #259

Merged
merged 3 commits into from Sep 1, 2021
Merged

Fix profiling in Python 3.10 #259

merged 3 commits into from Sep 1, 2021

Conversation

@jamadden
Copy link
Contributor

@jamadden jamadden commented Aug 31, 2021

This required some additional state management and several new tests.

Fixes #256.

jamadden added 3 commits Aug 30, 2021
The cframe member was always being initialized from the PyThreadState's root_cframe, which doesn't properly have use_tracing set, so new greenlets didn't get tracing.

Here, we do exactly what CPython's ceval.c does and stack-allocate a new CFrame object in g_initialstub and copy the use_tracing value from the currently executing thread state.

This seems to solve the tracing issue, but further tests are needed; none of the existing tests failed with the broken behaviour. In addition to adding an example from #256, we need to test that the use_tracing flag properly propagates up and down when a profiler is set inside a greenlet, likewise I've only been testing switch, need to test throw, and need to test several variations on calling g.run(): as a method, just a target function, an object assigned to a greenlet, etc.
This proves that propagating the tracer *upward* (to a parent greenlet) in Cpython 3.10 fails. However, 4287a40 did fix propagating it downward (to a new greenlet). Likely, propagating among sibling greenlets is also broken.
Because it's stack based, we need to store it separately.
@jamadden jamadden merged commit d70ab45 into master Sep 1, 2021
38 checks passed
@jamadden jamadden deleted the issue256 branch Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

1 participant