[Python] gRPC AsyncIO: Improve CompletionQueue polling performance

[aditya-1112]

Enable integer exception checking for performance improvement. This also resolves Asyncio segfaults with free-threaded Python.

Takes care of Cython performance hint:

performance hint: src/python/grpcio/grpc/_cython/_cygrpc/aio/completion_queue.pyx.pxi:126:22: Exception check after calling '_poll' will always require the GIL to be acquired. Possible solutions:	1. Declare '_poll' as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.	2. Use an 'int' return type on '_poll' to allow an error code to be returned. 

Details in https://cython.readthedocs.io/en/3.1.x/src/userguide/language_basics.html#error-return-values:

There’s a known performance pitfall when combining nogil and except * @cython.exceptval(check=True). In this case Cython must always briefly re-acquire the GIL after a function call to check if an exception has been raised. This can commonly happen with a function returning nothing (C void). Simple workarounds are to mark the function as noexcept if you’re certain that exceptions cannot be thrown, or to change the return type to int and just let Cython use the return value as an error flag (by default, -1 triggers the exception check).

[sergiitk]

I saw we call _handle_events below, which is aclassic python def method. Can we guarantee that it's not going to throw? Or noexcept only indicates that we explicitly raise from _poll itself? Not familiar with enough with Cython yet.

[sergiitk]

Hm. I think I just found a better explanation to the normal compiler warning we're getting:

There’s a known performance pitfall when combining nogil and except * @cython.exceptval(check=True). In this case Cython must always briefly re-acquire the GIL after a function call to check if an exception has been raised. This can commonly happen with a function returning nothing (C void). Simple workarounds are to mark the function as noexcept if you’re certain that exceptions cannot be thrown, or to change the return type to int and just let Cython use the return value as an error flag (by default, -1 triggers the exception check).
— https://cython.readthedocs.io/en/3.1.x/src/userguide/language_basics.html

I think unlike the compile regular warning, this doc implies that we shouldn't do either one or another, not both. Compare with the compile warning warning:

Exception check after calling '_poll' will always require the GIL to be acquired. Possible solutions:	1. Declare '_poll' as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.	2. Use an 'int' return type on '_poll' to allow an error code to be returned. 

The important part is

or to change the return type to int and just let Cython use the return value as an error flag (by default, -1 triggers the exception check).

Do I understand this correctly? Here's how I interpret this:

If we JUST change the return type from void to int, we'll still be able to throw the exception as before - without changing anything other then the method signature and adding return 1 on this line to indicate success.
Then Cython doesn't have this performance penalty because it now translate the exception we raised to exception to return -1. So later it know to acquire gil and handle the exception because the return is -1.

@sreenithi Does this make sense? Again, I have virtually no experience with Cython, so please verify this for me

[sreenithi]

ya this makes sense. We just tried it and verified that it works too. So he has changed the function accordingly. This also solves the problem with self._handle_events raising any errors. Because though it doesn't explicitly raise an error, there may be any run-time errors that might get missed out. Doing this will propagate that error successfully too.

Thanks for the suggestion!

[sreenithi]

However, I was thinking return 0 still makes sense as the last return value in case of success. Both in C/C++ or shell scripts, 0 is the usual convention for a successful return code right. So I was thinking it makes sense to keep it 0.

[sergiitk]

I'm ok with returning 0 here, especially since we're not checking the return value. I'd argue what to return gets a bit ambiguous in the cython context, which is in between C and python.

[aditya-1112]

Removed the 'noexcept' and declared the function as 'except -1' with integer return type, which signals an error via a -1 return value without the unsafe GIL acquisition on the return path, also allowing Python exceptions to be directly raised within the _poll function