On 22 September 2010 11:57, Stephane DROUARD <[email protected]> wrote:
> Lisandro Dalcin wrote:
>> > 1/ The simplest, but not the most optimized one: acquire/release the GIL
> within __Pyx_CppExn2PyErr().
>>
>> Why do you say (1) is not the most optimized one? (Sorry, I'm not good
>> at understanding issues with thread-based concurrency)
>
> Because it will acquire/release the GIL, even for functions that are not
> called
> without "with nogil".
> But maybe acquiring/releasing the GIL is not costy (I haven't checked).
>
I'm not sure about this, either...
>> > 2/ acquire/release the GIL in the catch clause only when "with nogil":
>> > try {foo();} catch(...) {PyGILState_STATE state = PyGILState_Ensure();
>> __Pyx_CppExn2PyErr(); PyGILState_Release(state); {__pyx_filename =
>> __pyx_f[0];
> __pyx_lineno = 6;
>> __pyx_clineno = __LINE__; goto __pyx_L6;}}
>> >
>>
>> How would this be different to (1) ?
>
> Because it can only acquires/releases the GIL when a function is called "with
> nogil" and not for the others (but more complex to implement as Cython needs
> to
> know the "with nogil" context.
>
>> > 3/ restore the thread in the catch clause:
>> > try {foo();} catch(...) { Py_BLOCK_THREADS __Pyx_CppExn2PyErr();
> {__pyx_filename =
>> __pyx_f[0]; __pyx_lineno = 6; __pyx_clineno = __LINE__; goto __pyx_L6;}}
>> > }
>> > /*finally:*/ {
>> > int __pyx_why;
>> > Py_BLOCK_THREADS // <<<<<<<<<<<<<<<<
>> > __pyx_why = 0; goto __pyx_L7;
>> > __pyx_L6: __pyx_why = 4; goto __pyx_L7;
>> > __pyx_L7:;
>> > switch (__pyx_why) {
>> > case 4: goto __pyx_L1_error;
>> > }
>> > }
>> > }
>> >
>>
>> And this one looks bad, I think you need: Py_BLOCK_THREADS
>> __Pyx_CppExn2PyErr(); Py_UNBLOCK_THREADS .. Am I right?
>>
>> What about doing this (ignore the line endings, all should be
>> generated in a single line)
>>
>> { PyThreadState *_save;
>> try {
>> Py_UNBLOCK_THREADS
>> foo();
>> Py_BLOCK_THREADS
>> } catch(...) {
>> Py_BLOCK_THREADS
>> __Pyx_CppExn2PyErr();
>> }
>>
>> Am I missing something? However, note that changing to this could be
>> not so easy...
>
> Because you may have several function calls within "with nogil":
> def bar():
> with nogil:
> foo()
> foo()
>
> and the generated code contains as many try/catch around the calls but only
> one
> thread save/restore:
> { PyThreadState *_save;
> Py_UNBLOCK_THREADS
> /*try:*/ {
> try {foo();} catch(...) {__Pyx_CppExn2PyErr(); {__pyx_filename =
> __pyx_f[0]; __pyx_lineno = 6; __pyx_clineno = __LINE__; goto __pyx_L6;}}
> try {foo();} catch(...) {__Pyx_CppExn2PyErr(); {__pyx_filename =
> __pyx_f[0]; __pyx_lineno = 7; __pyx_clineno = __LINE__; goto __pyx_L6;}}
> }
> /*finally:*/ {
> int __pyx_why;
> __pyx_why = 0; goto __pyx_L7;
> __pyx_L6: __pyx_why = 4; goto __pyx_L7;
> __pyx_L7:;
> Py_BLOCK_THREADS
> switch (__pyx_why) {
> case 4: goto __pyx_L1_error;
> }
> }
> }
>
OK, you are definitely right...
A last thing: in my previous mail I commented about fixing you
solution (3) like this:
Py_BLOCK_THREADS
__Pyx_CppExn2PyErr();
Py_UNBLOCK_THREADS
Is that right? Take into account that the /*finally:*/ block does
Py_BLOCK_THREADS...
If you can confirm that this works, I think you solution (3) with my
fix (in case it is fine) is be better option, right?
--
Lisandro Dalcin
---------------
CIMEC (INTEC/CONICET-UNL)
Predio CONICET-Santa Fe
Colectora RN 168 Km 472, Paraje El Pozo
Tel: +54-342-4511594 (ext 1011)
Tel/Fax: +54-342-4511169
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
