[Cython] [cython-users] What's up with PyEval_InitThreads() in python 2.7?

[mark florisson]

On 28 February 2012 13:50, Stefan Behnel <stefan_ml at behnel.de> wrote:
> mark florisson, 28.02.2012 12:20:
>> On 28 February 2012 10:53, Stefan Behnel wrote:
>>> mark florisson, 28.02.2012 11:28:
>>>> On 28 February 2012 10:25, Stefan Behnel wrote:
>>>>> mark florisson, 28.02.2012 11:16:
>>>>>> On 28 February 2012 09:54, Stefan Behnel wrote:
>>>>>>> I'm going to reimplement this, but not for 0.16 anymore, I'd say.
>>>>>>
>>>>>> That's ok, I fixed it to not acquire the GIL seeing that control flow
>>>>>> obsoletes None initialization. So you might as well move it into the
>>>>>> setup function if you care, the thing is that that would needlessly
>>>>>> acquire the GIL for the common case (when you have the GIL) in the
>>>>>> tests, so it might slow them down. It would be better to create a
>>>>>> __Pyx_RefNannySetupContextNogil() function wrapper. If you're not
>>>>>> running the code as a test the preprocessor would filter out this
>>>>>> bloat though, so it's really not a very big deal anyway.
>>>>>
>>>>> I was going to pass a constant flag into the macro that would let the C
>>>>> compiler do the right thing:
>>>>>
>>>>> """
>>>>> #ifdef WITH_THREAD
>>>>>  #define __Pyx_RefNannySetupContext(name, acquire_gil) \
>>>>>          if (acquire_gil) { \
>>>>>              PyGILState_STATE __pyx_gilstate_save = PyGILState_Ensure(); \
>>>>>              __pyx_refnanny = __Pyx_RefNanny->SetupContext((name), ...) \
>>>>>              PyGILState_Release(__pyx_gilstate_save); \
>>>>>          } else { \
>>>>>              __pyx_refnanny = __Pyx_RefNanny->SetupContext((name), ...) \
>>>>>          }
>>>>> #else
>>>>>  #define __Pyx_RefNannySetupContext(name, acquire_gil) \
>>>>>          __pyx_refnanny = __Pyx_RefNanny->SetupContext((name), ...)
>>>>> #endif
>>>>> """
>>>>>
>>>>> That also gets rid of the need to declare the "save" variable independently.
>>>>
>>>> I don't think that will work, I think the teardown re-uses the save variable.
>>>
>>> Well, it doesn't *have* to be the same variable, though.
>>>
>>> Speaking of that, BTW, there are a couple of places in the code (Nodes.py,
>>> from line 1500 on) that release the GIL, and some of them,
>>
>> Sorry, do you mean acquire?
>
> No, I meant release, at the exit points.
>
>
>>> specifically
>>> some error cases, look like they are using the wrong conditions to decide
>>> what they need to do in order to achieve that. I think this is most easily
>>> fixed by using the above kind of macro also for the refnanny cleanup call.
>>
>> If you could be more specific as to how the conditions might be wrong,
>> that would be helpful.
>
> I was thinking about the use of the "acquire_gil_for_refnanny_only" flag at
> the end, but maybe I got confused by the immense complexity of the function
> exit generation code, with its several places where the GIL can get
> acquired. I think this is worth some refactoring...
>
> In any case, the "acquire_gil_for_refnanny_only" flag can easily be used to
> trigger the necessary acquire-release code in the refnanny macros (as in
> the code above). That's at least a tiny first step to reducing this complexity.
>

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.

>> The conditions don't just pertain to refnanny,
>> nogil functions with with gil blocks and object arguments may also
>> incref their arguments
>
> I'm well aware of that.
>
>
>> The same goes for cleanup, it acquires the GIL to clean up any
>> temporaries and new references to function arguments. But there is
>> indeed room for optimization, e.g. if there are no new references to
>> object arguments and no error, then the GIL doesn't need to be
>> acquired. It's all rather subtle and tricky, but I think I wrote
>> decent tests at the time (but could always use more if the behaviour
>> were to be changed).
>
> I'm not currently intending to change the behaviour, just the generated code.
>
>
>> The cleanup could be further improved if each statement or each block
>> would do its own local cleanup. So any object temporaries would be
>> cleaned up before leaving the with gil block, etc. Anyway, changing
>> anything more than just the refnanny macros or functions would
>> probably have to wait until 0.16.1.
>
> Agreed. I'll see how my changes turn out, and then we can decide when to
> release them. Given that they don't really fix anything, just simplify the
> C code, I don't see a compelling enough reason to get them in for 0.16.

Sure, good luck :) I'll be moving for the next couple of days, so I'll
be rather preoccupied.

> Stefan
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel