chore: optimize dictionary access in strip_padding numpy#3994
Conversation
Skylion007
changed the title
| auto name = spec[0].cast<pybind11::str>(); | ||
| auto spec_fo = spec[1].cast<tuple>(); | ||
| for (auto kv_item : field_dict) { | ||
| auto name = reinterpret_borrow<pybind11::str>(kv_item.first); |
There was a problem hiding this comment.
IIUC this replaces obj.cast<pybind11::str>() — which does type checking — with reinterpret_borrow<pybind11::str> — which blindly assumes kv_item.first is a str. If that's true (?), and there is some upstream oversight/bug/API change, this could lead to a crash (relatively good) or a silent failure and time-consuming debugging.
There was a problem hiding this comment.
@rwgk Okay, I'll revert that part of the change. the dict_iterator returns an iterator of a std::pair which doesn't have a clean way of handling the steal / borrow distinction. I think you can call cast<> to a pyobject directly on it, but I don't know how it handles that code path to be honest.
There was a problem hiding this comment.
@rwgk I've reverted to the older dict api.
3 participants
Description
Further optimize strip_padding by making construction of field_descriptors faster by adding a ctor, using emplace, and trying to use the native C++ APIs for iterating through the dictionary.
Suggested changelog entry:
* optimize dictionary access in ``strip_padding`` for numpy