headers
Stefan Behnel | 1 Sep 08:38
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Robert Bradshaw wrote:
> Thanks. I think it looks pretty good, but I don't do much with the GIL
> so maybe someone with more experience on that end could comment.

I'll give it a try ASAP. lxml already implements things like this by hand, so
I'll have to rewrite a couple of functions the new way.

> I would propose renaming it to withGIL or something like that (or
> dropping the __ at the very least).

I also like withGIL.

Stefan
Permalink | Reply |
headers
Ulisses Furquim | 2 Sep 00:28

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

On 9/1/07, Stefan Behnel <stefan_ml@...> wrote:
>
> Robert Bradshaw wrote:
> > Thanks. I think it looks pretty good, but I don't do much with the GIL
> > so maybe someone with more experience on that end could comment.
>
> I'll give it a try ASAP. lxml already implements things like this by hand, so
> I'll have to rewrite a couple of functions the new way.

Great! Thanks!

> > I would propose renaming it to withGIL or something like that (or
> > dropping the __ at the very least).
>
> I also like withGIL.

Ok, that can be easily changed. :-)

Regards,

-- Ulisses
Permalink | Reply |
headers
Ulisses Furquim | 5 Sep 00:41

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

Hi,

On 9/1/07, Ulisses Furquim <ulisses.silva@...> wrote:
> On 9/1/07, Stefan Behnel <stefan_ml@...> wrote:
> > I also like withGIL.
>
> Ok, that can be easily changed. :-)

Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
thought that 'withGIL' was a better name to say we're gonna execute
that function _with the GIL_. IOW, it more clear we're gonna grab the
GIL and release it afterwards.

Best regards,

-- Ulisses

diff -r 6dccac108ab4 Compiler/Code.py
--- a/Compiler/Code.py	Sat Aug 25 19:01:14 2007 +0200
+++ b/Compiler/Code.py	Mon Sep 03 16:41:30 2007 -0300
@@ -284,6 +284,13 @@ class CCodeWriter:
         #	code = "((PyObject*)%s)" % code
         self.put_init_to_py_none(code, entry.type)

+    def put_py_gil_state_ensure(self, cname):
+        self.putln("PyGILState_STATE %s;" % cname)
+        self.putln("%s = PyGILState_Ensure();" % cname)
+
+    def put_py_gil_state_release(self, cname):
(Continue reading)

Permalink | Reply |
headers
Stefan Behnel | 5 Sep 09:35
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Ulisses Furquim wrote:
> Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
> thought that 'withGIL' was a better name to say we're gonna execute
> that function _with the GIL_. IOW, it more clear we're gonna grab the
> GIL and release it afterwards.

Thanks, it applies cleanly for me. I will give it some testing.

Stefan
Permalink | Reply |
headers
Stefan Behnel | 10 Sep 23:08
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Stefan Behnel wrote:
> Ulisses Furquim wrote:
>> Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
>> thought that 'withGIL' was a better name to say we're gonna execute
>> that function _with the GIL_. IOW, it more clear we're gonna grab the
>> GIL and release it afterwards.
> 
> Thanks, it applies cleanly for me. I will give it some testing.

Hmm, the first problem I encountered was that it changes the signature that
Cython sees. So I get this error when assigning a "withGIL" function to a
typed function slot:

========================================================
Cannot assign type 'void ((void (*),char (*),char (*),char (*)) withGIL)' to
'endElementNsSAX2Func'
========================================================

Your patch explicitly checks this part of the signature when comparing
function types, but actually, I can't see why we should do that. There is no
difference in the signature of a function running with GIL and another running
without. But honouring the "withGIL" in comparisons breaks assignments between
the two. So we can avoid a lot of hassle if we ignore the "withGIL" in type
comparisons. The updated patch does this (it just removes the explicit tests).

As you can infer from the above error message, I was implementing a SAX
handler. The parser *might* execute outside the GIL context, but the callbacks
call back into Python, so they require the GIL. Your patch kept me from
writing twice the number of functions, one for each callback to grab the GIL
(Continue reading)

Permalink | Reply |
headers
Ulisses Furquim | 11 Sep 15:36

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

Hi,

On 9/10/07, Stefan Behnel <stefan_ml@...> wrote:
> Stefan Behnel wrote:
> > Ulisses Furquim wrote:
> >> Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
> >> thought that 'withGIL' was a better name to say we're gonna execute
> >> that function _with the GIL_. IOW, it more clear we're gonna grab the
> >> GIL and release it afterwards.
> >
> > Thanks, it applies cleanly for me. I will give it some testing.
>
> Hmm, the first problem I encountered was that it changes the signature that
> Cython sees. So I get this error when assigning a "withGIL" function to a
> typed function slot:
>
> ========================================================
> Cannot assign type 'void ((void (*),char (*),char (*),char (*)) withGIL)' to
> 'endElementNsSAX2Func'
> ========================================================

Yes, it was made on purpose so we have to annotate both the function
and the places where you have to pass them.

> Your patch explicitly checks this part of the signature when comparing
> function types, but actually, I can't see why we should do that. There is no
> difference in the signature of a function running with GIL and another running
> without. But honouring the "withGIL" in comparisons breaks assignments between
> the two. So we can avoid a lot of hassle if we ignore the "withGIL" in type
> comparisons. The updated patch does this (it just removes the explicit tests).
(Continue reading)

Permalink | Reply |
headers
Robert Bradshaw | 11 Sep 11:22
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

On Sep 10, 2007, at 2:08 PM, Stefan Behnel wrote:

> Stefan Behnel wrote:
>> Ulisses Furquim wrote:
>>> Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
>>> thought that 'withGIL' was a better name to say we're gonna execute
>>> that function _with the GIL_. IOW, it more clear we're gonna grab  
>>> the
>>> GIL and release it afterwards.
>>
>> Thanks, it applies cleanly for me. I will give it some testing.
>
> Hmm, the first problem I encountered was that it changes the  
> signature that
> Cython sees. So I get this error when assigning a "withGIL"  
> function to a
> typed function slot:
>
> ========================================================
> Cannot assign type 'void ((void (*),char (*),char (*),char (*))  
> withGIL)' to
> 'endElementNsSAX2Func'
> ========================================================
>
> Your patch explicitly checks this part of the signature when comparing
> function types, but actually, I can't see why we should do that.  
> There is no
> difference in the signature of a function running with GIL and  
> another running
> without. But honouring the "withGIL" in comparisons breaks
(Continue reading)

Permalink | Reply |
headers
Ulisses Furquim | 11 Sep 15:42

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

Hi,

On 9/11/07, Robert Bradshaw <robertwb@...> wrote:
> On the other hand, by requiring the two signature to match exactly,
> it is clear exactly what is happening. If you don't require them to
> match, does it do withGIL if only the pxd file/only the pyx file has
> that flag? Would overriding functions in descendant classes have to
> match, or would some be with the GiL and some without?

It's specific to the function declaration. If it's not there then
there's no effect.

> > As you can infer from the above error message, I was implementing a
> > SAX
> > handler. The parser *might* execute outside the GIL context, but
> > the callbacks
> > call back into Python, so they require the GIL. Your patch kept me
> > from
> > writing twice the number of functions, one for each callback to
> > grab the GIL
> > and a second one to do the actual work. That definitely got me
> > convinced.
>
> I think it's a great idea too.

:-)

> I think it belongs after rather than before, and it might be more
> pythonic to even put a space in there, i.e.
>
(Continue reading)

Permalink | Reply |
headers
Stefan Behnel | 11 Sep 12:32
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Robert Bradshaw wrote:
> pythonic to even put a space in there, i.e.
> 
> cdef int test(int param) with GIL:

Here's a patch against the last version I sent that does that. It accepts a
list of comma separated identifiers after "with", but currently only allows "GIL".

I implemented the list right in the parser. Maybe we should move it somewhere
else, but that's not a major concern for me.

Stefan
Attachment (cython-with-GIL.patch): text/x-diff, 2618 bytes
_______________________________________________
Cython-dev mailing list
Cython-dev@...
https://lists.berlios.de/mailman/listinfo/cython-dev
Permalink | Reply |
headers
Ulisses Furquim | 11 Sep 15:54

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

Hi,

On 9/11/07, Stefan Behnel <stefan_ml@...> wrote:
> Here's a patch against the last version I sent that does that. It accepts a
> list of comma separated identifiers after "with", but currently only allows "GIL".

Nice. :-)

> I implemented the list right in the parser. Maybe we should move it somewhere
> else, but that's not a major concern for me.

Maybe in Scanning.py together with the list of keywords?

Regards,

-- Ulisses
Permalink | Reply |
headers
Stefan Behnel | 11 Sep 21:37
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Ulisses Furquim wrote:
> On 9/11/07, Stefan Behnel <stefan_ml@...> wrote:
>> Here's a patch against the last version I sent that does that. It accepts a
>> list of comma separated identifiers after "with", but currently only allows "GIL".
> 
> Nice. :-)
> 
>> I implemented the list right in the parser. Maybe we should move it somewhere
>> else, but that's not a major concern for me.
> 
> Maybe in Scanning.py together with the list of keywords?

Sure, why not. Here's a complete patch for the new feature. I also submitted
it to the bug tracker (including an hg export).

https://bugs.launchpad.net/cython/+bug/138963

Stefan
Attachment (cython-with-GIL-simple.patch): text/x-diff, 8 KiB
_______________________________________________
Cython-dev mailing list
Cython-dev@...
https://lists.berlios.de/mailman/listinfo/cython-dev
Permalink | Reply |
headers
Stefan Behnel | 11 Sep 11:51
Picon
Favicon

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release


Robert Bradshaw wrote:
> On Sep 10, 2007, at 2:08 PM, Stefan Behnel wrote:
> 
>> Stefan Behnel wrote:
>>> Ulisses Furquim wrote:
>>>> Here is the patch with '__grabGIL' renamed to 'withGIL'. I also
>>>> thought that 'withGIL' was a better name to say we're gonna execute
>>>> that function _with the GIL_. IOW, it more clear we're gonna grab the
>>>> GIL and release it afterwards.
>>>
>>> Thanks, it applies cleanly for me. I will give it some testing.
>>
>> Hmm, the first problem I encountered was that it changes the signature
>> that
>> Cython sees. So I get this error when assigning a "withGIL" function to a
>> typed function slot:
>>
>> ========================================================
>> Cannot assign type 'void ((void (*),char (*),char (*),char (*))
>> withGIL)' to
>> 'endElementNsSAX2Func'
>> ========================================================
>
> On the other hand, by requiring the two signature to match exactly, it
> is clear exactly what is happening. If you don't require them to match,
> does it do withGIL if only the pxd file/only the pyx file has that flag?

Didn't test, but I don't think so (and I hope it doesn't). It's specific to
the implementation of the function itself, so if it's not declared on the
(Continue reading)

Permalink | Reply |
headers
Ulisses Furquim | 11 Sep 15:48

Re: [Pyrex] Callbacks from threads and PyGILState_Ensure/PyGILState_Release

Hi,

On 9/11/07, Stefan Behnel <stefan_ml@...> wrote:
> > Would overriding functions in descendant classes have to match, or would
> > some be with the GiL and some without?
>
> Even if we implement an interface or override a method, the question still is:
> does this specific *implementation* require the GIL to execute? You can always
> come up with cases where you can reimplement a method in plain C code that
> does not require the GIL at all, or you can implement it in Python-like code
> that will crash if the GIL is not there. And the interface (or signature)
> doesn't necessarily have to change.

I think you've made a good point about not having this option in the
function signature.

> >> The second thing is that I'm not sure where I prefer the "withGIL".
> >
> > I think it belongs after rather than before, and it might be more
> > pythonic to even put a space in there, i.e.
> >
> > cdef int test(int param) with GIL:
>
> That looks good. Also, the fact that it comes after the actual signature makes
> it clearer that it *is* an implementation detail.

Agreed.

> > with is a reserved keyword, but I don't know about GIL... This would
> > allow for future extensions (though I can't think of any now) to say a
(Continue reading)

Permalink | Reply |

Gmane