cython icon indicating copy to clipboard operation
cython copied to clipboard

[BUG] Getting "performance hint: ..."

Open YoSTEALTH opened this issue 1 year ago • 2 comments

Describe the bug

I keep getting these "performance hint" bellow.

Goal is to run io_uring_queue_init with nogil, since nogil required other functions to be nogil as well I have set trap_error to be nogil as well and only trigger error when return value is < 0, why with gil: block is used.

I don't want to use noexcept for trap_error since half the function does need to trigger error.

Code to reproduce the behaviour:

# liburing.pyx
cpdef int io_uring_queue_init(unsigned int entries, io_uring ring, unsigned int flags=0) nogil:
    return trap_error(io_uring_queue_init_c(entries, ring.ptr, flags))
# error.pyx
cpdef inline int trap_error(int no) nogil:
    if no >= 0:
        return no
    with gil:
        raise_error(no)


cdef inline void raise_error(signed int no=-1) except *:
    no = -errno or no
    cdef str error = strerror(-no).decode()
    raise OSError(-no, error)
performance hint: src/liburing/liburing.pyx:61:21: Exception check after calling 'trap_error' will always
require the GIL to be acquired. Declare 'trap_error' as 'noexcept' if you control the definition and
you're sure you don't want the function to raise
exceptions.

Expected behaviour

No response

OS

Linux

Python version

3.12

Cython version

3.0.8

Additional context

No response

YoSTEALTH avatar Feb 15 '24 05:02 YoSTEALTH

I can't reproduce with the code given here. To make it compile I slightly modified it to the below:

# liburing.pyx

from error cimport trap_error

cpdef int io_uring_queue_init(int x) nogil:
    return trap_error(x)
# error.pxd

cpdef int trap_error(int no) nogil
# error.pyx

from libc.errno cimport errno
from libc.string cimport strerror

cpdef inline int trap_error(int no) nogil:
    if no >= 0:
        return no
    with gil:
        raise_error(no)


cdef inline void raise_error(signed int no=-1) except *:
    no = -errno or no
    cdef str error = strerror(-no).decode()
    raise OSError(-no, error)

I get no performance warnings, which I believe is correct.

da-woods avatar Feb 15 '24 07:02 da-woods

@da-woods I had the same code before(tried all the combination). When you first compile the code you get the performance hint: warning, the second time you don't get the message because the ./build/ folder needs to be deleted and compile again fresh to see the message again.

It kept tripping me as well...

p.s. you can see actual code at: https://github.com/YoSTEALTH/Liburing/tree/master/src/liburing

YoSTEALTH avatar Feb 15 '24 17:02 YoSTEALTH

@da-woods have you tried the code after deleting ./build/?

YoSTEALTH avatar Feb 27 '24 21:02 YoSTEALTH

Sorry you're right. There is a ~warning~ performance hint.

That looks to be by design and that we don't assume the -1 return value from functions declared in pxd files:

https://github.com/cython/cython/blob/03d982be010b7b94a3c3317c462b2b803ce916af/Cython/Compiler/Nodes.py#L729

I think unfortunately that is correct, and that you can compile something with:

# in the .pxd
cpdef int trap_error(int no) nogil
# in the .pyx file
cpdef inline int trap_error(int no) except -2 nogil:

so if you just see the pxd file you can't reason about the error code.

That suggests we need to improve the performance hint to tell you that since I don't think we can change the behaviour easily.

It might also we worth warning about "exception value in pyx but not in pxd" but I'm not sure about that.

da-woods avatar Feb 28 '24 07:02 da-woods

That didn't work, was still getting the same message. So, I tried random stuff and this seem to work:


# error.pxd
cpdef int trap_error(int no) except -1 nogil
# error.pyx
cpdef inline int trap_error(int no) except -1 nogil:

Don't even know whats going on! But happy the message is gone :)

YoSTEALTH avatar Feb 28 '24 08:02 YoSTEALTH

Your solution is right. The code I posted was an explanation, not something you should do

da-woods avatar Feb 28 '24 08:02 da-woods

@da-woods Cool, Thank your very much for your amazing support.

YoSTEALTH avatar Feb 28 '24 08:02 YoSTEALTH

I'm going to reopen this because I think we could improve the text of the performance hints here.

da-woods avatar Feb 28 '24 21:02 da-woods

@da-woods For io_uring_enter since trap_error is being used, also by other functions, getting lots of these messages:


performance hint: src/liburing/syscall.pxd:7:24: No exception value declared for 'io_uring_enter' in pxd file.
Users cimporting this function and calling it without the gil will always require an exception check.
Suggest adding an explicit exception value.

tested with 3.0.10 and 3.1.0a0

YoSTEALTH avatar Mar 30 '24 21:03 YoSTEALTH