Drop test for tracing and inline coroutine creation #16
Conversation
Hummm, I thought I already rebased to pick that. What are you running to check for refleaks? I checked and we were not leaking anything before :S |
|
Ah, yes, we need to pick up the changes in master. I will merge master into the PR to pick up the refleak fixes. |
|
Done in commit 832bccf |
| PyCodeObject *code = (PyCodeObject*)PyFunction_GET_CODE(function); | ||
| int is_coro = code->co_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR); | ||
| inline_call = (is_coro || cframe.use_tracing) ? 0 : 1; | ||
| STACK_SHRINK(oparg + 1); |
There was a problem hiding this comment.
I am fine with this but I find this a bit more difficult to read than the previous code. This has two conditionals nested and is not immediately clear what's the unoptimized path
There was a problem hiding this comment.
I don't think there is an optimized or unoptimized path here. The specialized versions can worry about fast paths.
Python/ceval.c
Outdated
| PUSH(res); | ||
| if (res == NULL) { | ||
| goto error; | ||
| } |
There was a problem hiding this comment.
The control flow here is quite confusing. This is falling over to the DISPATH() outside the conditional, which is quite far from here.
What's the advantage of doing this here? Is this because you plan to also inline coroutines?
There was a problem hiding this comment.
This PR already inlines coroutine creation. I've moved the code around a bit to make it clearer.
|
I left two things but we can merge this, I think it makes sense but I would like more insight into what you want to do here, to not scope creep this PR a lot |
|
Thanks for the PR @markshannon ! |
I've not attempted cleanup of the reference counting yet due to existing leaks.
These two commits are clean up before doing that.
The
simple_cframebranch leaks references.Do you need to merge in main to get python#28493?
My plan is:
make_corosteal its argument's references.stealparameter