On 28 February 2012 21:08, mark florisson <[email protected]> wrote: > On 28 February 2012 20:19, Stefan Behnel <[email protected]> wrote: >> Stefan Behnel, 28.02.2012 20:58: >>> mark florisson, 28.02.2012 16:35: >>>> Basically, the cleanup code only needs a matching release because the >>>> corresponding acquire is in EnsureGILNode, which wraps the function >>>> body in case of a nogil function with a 'with gil' block. Any changes >>>> to the conditions in FuncDefNode will have to be reflected by the code >>>> that does that wrapping. Changing the refnanny macro for the cleanup >>>> code will not avail anything, as the GIL is already ensured. >>> >>> Regarding the "with gil" code, ISTM that the "finally" code in the with_gil >>> test is being duplicated. I noticed this when I moved the refnanny's GIL >>> state into a block local variable and that broke the C code. Basically, the >>> with-gil block had declared the variable in its own block, but was then >>> trying to access that variable in a second finally clause, further down and >>> outside of the with-gil block. >> >> Hmm, guess I got confused again... >> >> So, the code is this: >> >> """ >> /*finally:*/ { >> int __pyx_why; >> __pyx_why = 0; goto __pyx_L8; >> __pyx_L7: __pyx_why = 4; goto __pyx_L8; >> __pyx_L8:; >> #ifdef WITH_THREAD >> PyGILState_Release(__pyx_gilstate_save); >> #endif >> switch (__pyx_why) { >> case 4: goto __pyx_L4; >> } >> } >> } >> } >> /*finally:*/ { >> int __pyx_why; >> __pyx_why = 0; goto __pyx_L5; >> __pyx_L4: __pyx_why = 4; goto __pyx_L5; >> __pyx_L5:; >> #ifdef WITH_THREAD >> __pyx_gilstate_save = PyGILState_Ensure(); >> #endif >> switch (__pyx_why) { >> case 4: goto __pyx_L1_error; >> } >> } >> >> goto __pyx_L0; >> __pyx_L1_error:; >> __Pyx_XDECREF(__pyx_t_1); >> __Pyx_XDECREF(__pyx_t_2); >> __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception", >> __pyx_clineno, __pyx_lineno, __pyx_filename); >> __pyx_L0:; >> #ifdef WITH_THREAD >> PyGILState_Release(__pyx_gilstate_save); >> #endif >> } >> """ >> >> The first "finally" block is inside of the with-gil block, whereas the >> second is outside. In the second, the GIL is reacquired, and I guess that's >> for cleaning up temps in the error case, right? I see the problem that it >> can't be acquired after jumping to the exit labels (error/return) in the >> current code layout, but wouldn't it make sense to generate this code >> instead (starting at the second finally clause): > > This code could certainly be optimized, I just never got around to > implementing it. > >> """ >> /*finally:*/ { >> int __pyx_why; >> __pyx_why = 0; goto __pyx_L5; >> __pyx_L4: __pyx_why = 4; goto __pyx_L5; >> __pyx_L5:; >> switch (__pyx_why) { >> case 4: goto __pyx_L1_error; >> } >> } >> >> goto __pyx_L0; >> __pyx_L1_error:; >> #ifdef WITH_THREAD >> __pyx_gilstate_save = PyGILState_Ensure(); >> #endif >> __Pyx_XDECREF(__pyx_t_1); >> __Pyx_XDECREF(__pyx_t_2); >> __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception", >> __pyx_clineno, __pyx_lineno, __pyx_filename); >> #ifdef WITH_THREAD >> PyGILState_Release(__pyx_gilstate_save); >> #endif >> return; /* <- error return here! */ >> __pyx_L0:; >> } >> """ >> >> Meaning: split the return code paths and let them independently acquire the >> GIL if needed? That would at least relieve the outer finally from having to >> care about the GIL and having to rely on "someone" to declare the GIL >> variable and eventually release it. > > (Sorry, dinner). In this case, that would be valid. Normally though, > the exception case should fall through to the normal case, so I think > what you'd want is > > ... > goto L0; > L1: > PyGILState_Ensure(); > /* error code */ > __pyx_r = ...; > goto L2; > L0: > PyGILState_Ensure(); > goto L2: > /* normal code */ > return __pyx_r; > }
And there should be a release just before the return of course :) > If the entire body is a 'with gil' block, it'd probably be easiest to > transform the nogil function into a 'with gil' function. If you want > to optimize for "paths going from with gil blocks directly into > function cleanup code" (i.e., no intermediate nogil try/finally or > nested with gil blocks), then you could use two more labels in the > code above that bypass the acquire. > >> Or, alternatively, add an additional exit path to the finally block that >> jumps to a label that acquires the GIL before going on to the original >> label. Something along those lines... >> >> Stefan >> _______________________________________________ >> cython-devel mailing list >> [email protected] >> http://mail.python.org/mailman/listinfo/cython-devel _______________________________________________ cython-devel mailing list [email protected] http://mail.python.org/mailman/listinfo/cython-devel
