[QUESTION] How to efficiently stop iteration?

[kenoslund-google]

I defined an iterable class in pybind11 by creating a __getitem__ method which throws a std::out_of_range exception if the index is out of range. While this successfully stops the iteration in python (without raising a python exception), it appears that stopping the iteration takes about 100us vs 1us for a native implementation, so ~100x slower. I tried the following things:

A. PyErr_SetString(PyExc_StopIteration, "..."); throw error_already_set();: takes ~25us, so ~4x faster than std::out_of_range but still ~25x slower than the native version.
B. PyErr_SetString(PyExc_StopIteration, "..."); and then propagating the error manually, without throwing a C++ exception. This takes ~5us, so ~20x faster than std::out_of_range and "only" ~5x slower than the native version.

However, option B requires returning a NULL PyObject to python, which the dispatcher doesn't like- it triggers the error here: https://github.com/pybind/pybind11/blob/master/include/pybind11/pybind11.h#L924-L928. In order to run option 2, I have to patch those lines to check PyErr_Occurred() and only set an error if none is already set.

so, my questions:

1. Is there any equally or more efficient way to define an iterable object in C++ using pybind11 than __getitem__ and option B above?
2. If the answer to that is "no", can cpp_function::dispatcher be patched to allow setting a python error directly, without throwing a C++ exception?

[bstaletic]

Setting an error, but not throwing error_already_set is not always safe, but it is the fastest thing you can do, as far as I know. If your patch doesn't break the tests, make a pull request.

[YannickJadoul]

As far as I know, pybind11 really deeply depends on exceptions by design, though?

[bstaletic]

That's why I said it's not always safe to set an error and not throw. You can get in trouble.

[kenoslund-google]

Patching pybind11 to allow setting an error but not throwing an error_already_set exception causes 2 failures:

in test_class:

_________________________ test_error_after_conversions _________________________

    def test_error_after_conversions():
        with pytest.raises(TypeError) as exc_info:
            m.test_error_after_conversions("hello")
>       assert str(exc_info.value).startswith(
            "Unable to convert function return value to a Python type!"
        )
E       AssertionError: assert False
E        +  where False = <built-in method startswith of str object at 0x7fe9cb80a558>('Unable to convert function return value to a Python type!')
E        +    where <built-in method startswith of str object at 0x7fe9cb80a558> = 'Unregistered type : test_submodule_class_(module_&)::NotRegistered'.startswith
E        +      where 'Unregistered type : test_submodule_class_(module_&)::NotRegistered' = str(TypeError('Unregistered type : test_submodule_class_(module_&)::NotRegistered',))
E        +        where TypeError('Unregistered type : test_submodule_class_(module_&)::NotRegistered',) = <ExceptionInfo TypeError tblen=1>.value

test_class.py:375: AssertionError

in test_stl:

_________________________ test_missing_header_message __________________________

    def test_missing_header_message():
        """Trying convert `list` to a `std::vector`, or vice versa, without including
        <pybind11/stl.h> should result in a helpful suggestion in the error message"""
        import pybind11_cross_module_tests as cm
    
        expected_message = (
            "Did you forget to `#include <pybind11/stl.h>`? Or <pybind11/complex.h>,\n"
            "<pybind11/functional.h>, <pybind11/chrono.h>, etc. Some automatic\n"
            "conversions are optional and require extra headers to be included\n"
            "when compiling your pybind11 module."
        )
    
        with pytest.raises(TypeError) as excinfo:
            cm.missing_header_arg([1.0, 2.0, 3.0])
        assert expected_message in str(excinfo.value)
    
        with pytest.raises(TypeError) as excinfo:
            cm.missing_header_return()
>       assert expected_message in str(excinfo.value)
E       AssertionError: assert 'Did you forget to `#include <pybind11/stl.h>`? Or <pybind11/complex.h>,\n<pybind11/functional.h>, <pybind11/chrono.h>...ome automatic\nconversions are optional and require extra headers to be included\nwhen compiling your pybind11 module.' in 'Unregistered type : std::__u::vector<float, std::__u::allocator<float> >'
E        +  where 'Unregistered type : std::__u::vector<float, std::__u::allocator<float> >' = str(TypeError('Unregistered type : std::__u::vector<float, std::__u::allocator<float> >',))
E        +    where TypeError('Unregistered type : std::__u::vector<float, std::__u::allocator<float> >',) = <ExceptionInfo TypeError tblen=1>.value

test_stl.py:237: AssertionError

I haven't had time to look into them in detail.

Setting an error, but not throwing error_already_set is not always safe,

Can you elaborate? what could go wrong?

[kenoslund-google]

If C++ exceptions and pybind11's handling of them is going to incur that much overhead, it would be nice to make using them optional, especially in cases where the exception is used to indicate a normal/expected condition and there's no real error, such as loop termination.

[bstaletic]

Can you elaborate? what could go wrong?

Well, random things can break because of the assumption that PyErr_SetString() is always followed by throw py::error_already_set. You've seen a broken test. Not throwing error_already_set is basically breaking the contract.

[kenoslund-google]

I might be able to get around those failures- I haven't had time to try.

alternatively, how many cases are an exception used to indicated a normal/expected condition? loop termination is the only one that immediately comes to my mind. maybe we just handle that as a special case, rather than more generally allowing setting exceptions without throwing a C++ exception?

[YannickJadoul]

Not throwing error_already_set is basically breaking the contract.

That's basically taking the words out of my mouth, yes. pybind11 builds on the assumption that after (C)Python sets an error (and the C API maybe returns an error code), error_already_set is thrown. It's a pretty fundamental assumption throughout pybind11, as far as I know.

I might be wrong (feel free to prove me wrong), but I also don't think CPython likes it if you call more parts of the C API when an error is already set? So I think this is the underlying reason that pybind11 goes for exceptions, exactly because they do traverse the whole stack. So maybe pybind11 isn't the right tool for lightning-fast exceptions, here. (IIRC, Boost.Python basically followed the same approach, or not?)

Can you elaborate? what could go wrong?

So, to answer this: I don't know, but calls into the C API with the error set are a good candidate. But (worrying me the most) since you're breaking a pretty fundamental contract: potentially lots of unforeseen things. In this regard, I'm wondering how extensive pybind11's test suite is, as well.

That being said: of course it would be nice if pybind11 would be faster and could work without exceptions. And of course, this is just for exceptions in customly-bound functions and you're not proposing to remove all exceptions (you aren't, are you?). But exceptions have always been part of the pybind11 package-deal (again, as far as I know; I have not helped design pybind11, I'm just an observer afterwards, seeing how things are currently done), so I'd be surprised if cutting them out is an easy thing to do (or even something to be attempted).

[YannickJadoul]

Out of curiosity: the slowdown in the numbers you mentioned is quite big, but does it really matter if your returning to Python anyway? Python isn't exactly known to shave off too many micro and milliseconds to run, either?

[bstaletic]

So, to answer this: I don't know, but calls into the C API with the error set are a good candidate.

C API can do 3 things:

• Ignore the previous error and continue, possibly overwriting the set error with a new one.
• Return early.
• Fail an assertion.

That's the general case. @kenoslund-google isn't the first one wishing to set an error without throwing. @mosra was, at least to my knowledge.

https://blog.magnum.graphics/announcements/introducing-python-bindings/#the-less-than-great-aspects-of-pybind11
#1853

[YannickJadoul]

That being said, the current thing being mentioned is a reasonably local case to eliminate exceptions (only when returning from a function to Python, I believe?), so I'm not claiming it's impossible to make this specific case work.

[kenoslund-google]

yep, that blog post observes exactly the same thing I did, and pull request #1853 is exactly what I tried.

I don't care that much about performance when an exception is thrown because of a real error. But stop iteration isn't an actual error- it's expected and happens every time you iterate to the end of a list. That's the only case that I would like to optimize for, and it seems like other people have the same request.

What if we created some kind of static global object whose only purpose is to say "Allow a python exception to be set without throwing a C++ exception", and then this could be returned by functions which implement __getitem__? It would probably have to a python object, so functions using it would have to return a py::object/handle rather than a native C++ type, but that's not too bad (they just have to call py::cast manually, rather than letting pybind11 do it for them). Then all existing code would continue to behave exactly the way it currently does, but the user would have the option to say "yes, I really do mean to set an error without throwing a C++ exception". And of course at that point it's their responsibility to do it right.

[bstaletic]

A py::object at global scope or with static lifetime is a bad idea. You can't initialize it before the interepter has started and you can't clean it up, ever, which doesn't really fit in with how python objects are supposed to be handled. There are python singletons, but even those don't have a static life time.

[kenoslund-google]

What do you think of the general idea of somehow flagging "Yes, I really do mean to set a python exception without throwing a C++ exception, so allow it" to avoid breaking any existing uses? If so, how could we implement that flag?

[bstaletic]

If we can't make it work otherwise, the flag would be fine with me, but I don't know how to implement it straight away.