Fixed GIL regressions with Cython 3

[WillAyd]

ref #55179

The main issue was that the functions returning double were not marked noexcept, so in Cython 3.0 these now explicitly check the global Python error indicator, which requires a reacquisition of the GIL

The changes to to_fw_string were to fix warnings generated by Cython that subsequently appeared:

 [2/4] Compiling Cython source /home/willayd/clones/pandas/pandas/_libs/parsers.pyx
  performance hint: /home/willayd/clones/pandas/pandas/_libs/parsers.pyx:1622:5: Exception check on '_to_fw_string_nogil' will always require the GIL to be acquired.
  Possible solutions:
        1. Declare the function 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 the function to allow an error code to be returned.
  performance hint: /home/willayd/clones/pandas/pandas/_libs/parsers.pyx:1617:27: Exception check will always require the GIL to be acquired.
  Possible solutions:
        1. Declare the function 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 the function to allow an error code to be returned.
  performance hint: /home/willayd/clones/pandas/pandas/_libs/parsers.pyx:1707:42: Exception check will always require the GIL to be acquired. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
  performance hint: /home/willayd/clones/pandas/pandas/_libs/parsers.pyx:1731:38: Exception check will always require the GIL to be acquired. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.

We maybe shouldn't just be swallowing errors like this, but that is a different problem for a different day...for now this should get us back to 0.29 performance

[rhshadrach]

I'm seeing 4 more instances of nogil without noexcept, 3 in parsers.pyx and one in offsets.pyx. Add these as well?

[WillAyd]

Which functions are you looking at? In theory I think we only want to do this for non-integral return types. The integral returning functions should be checking the error sentinel first before trying to acquire the GIL, which I think is more reasonable than noexcept

[lithomas1]

Do we have to put noexcept even when there's no nogil?

[WillAyd]

Whether or not the GIL is in play you will now be doing a PyErr_Occurred() after each invocation, which causes Python runtime interaction and will be "slower". With that said I didn't benchmark this particular function - just reset it to the original .29 behavior. I'll try to run some tests later though to see if it matters; it would be better to check for exceptions in case the numpy call fails, or this gets refactored in the future.

Generally I'd consider the use of void functions in our Cython code bad practice, unless failure within the function absolutely cannot be handled (ex: in destructors). It would be better to always return an int for error handling, which Cython can check without Python runtime interaction

[lithomas1]

Thanks Will.

[mroeschke]

Thanks @WillAyd

[rhshadrach]

Based on #55650 (comment), don't we not want noexcept here? It's still not clear to me why we don't though.

[WillAyd]

Good callout - this PR just got us back to 0.29 behavior but we could do better with the error handling. The problem is the body of the function returns 1 on error and doesn't set a Python exception. We at least would need to return -1 and I think set a Python exception in an ideal world

[rhshadrach]

Which functions are you looking at? In theory I think we only want to do this for non-integral return types. The integral returning functions should be checking the error sentinel first before trying to acquire the GIL, which I think is more reasonable than noexcept

They were indeed all integral return types. Can you explain further the italic bit above? Is the "should be checking" bit done by the caller? Isn't whether an exception was raised checked before we get back to the caller?