Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: cache intermediate str objects in PyObject_GetAttr calls #106

Merged
merged 2 commits into from
Nov 20, 2023
Merged

perf: cache intermediate str objects in PyObject_GetAttr calls #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1839c3
perf: cache intermediate `str` objects in `PyObject_GetAttr` calls
XuehaiPan Nov 18, 2023
419eae1
docs(CHANGELOG): update CHANGELOG.md
XuehaiPan Nov 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- Cache intermediate `str` objects in `PyObject_GetAttr` calls by [@XuehaiPan](https://github.com/XuehaiPan) in [#106](https://github.com/metaopt/optree/pull/106).
- Install `clang-format` and `clang-tidy` from PyPI by [@XuehaiPan](https://github.com/XuehaiPan) in [#107](https://github.com/metaopt/optree/pull/107).
- Also check `_make` and `_asdict` in function `is_namedtuple_class` by [@XuehaiPan](https://github.com/XuehaiPan) in [#105](https://github.com/metaopt/optree/pull/105).

Expand Down
50 changes: 38 additions & 12 deletions include/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,30 @@ inline void HashCombine(py::ssize_t& seed, const T& v) { // NOLINT[runtime/refe
constexpr bool NONE_IS_LEAF = true;
constexpr bool NONE_IS_NODE = false;

// NOTE: Use raw pointers to leak the memory intentionally to avoid py::object deallocation and
// garbage collection.
#define Py_Declare_ID(name) \
inline PyObject* Py_ID_##name() { \
static PyObject* ptr = (new py::str{#name})->ptr(); \
return ptr; \
}

#define Py_Get_ID(name) (Py_ID_##name())

Py_Declare_ID(__module__); // type.__module__
Py_Declare_ID(__qualname__); // type.__qualname__
Py_Declare_ID(__name__); // type.__name__
Py_Declare_ID(sort); // list.sort
Py_Declare_ID(copy); // dict.copy
Py_Declare_ID(default_factory); // defaultdict.default_factory
Py_Declare_ID(maxlen); // deque.maxlen
Py_Declare_ID(_fields); // namedtuple._fields
Py_Declare_ID(_make); // namedtuple._make
Py_Declare_ID(_asdict); // namedtuple._asdict
Py_Declare_ID(n_fields); // structseq.n_fields
Py_Declare_ID(n_sequence_fields); // structseq.n_sequence_fields
Py_Declare_ID(n_unnamed_fields); // structseq.n_unnamed_fields

#define PyCollectionsModule (ImportCollections())
#define PyOrderedDictTypeObject (ImportOrderedDict())
#define PyDefaultDictTypeObject (ImportDefaultDict())
Expand Down Expand Up @@ -332,7 +356,7 @@ inline bool IsNamedTupleClassImpl(const py::handle& type) {
// We can only identify namedtuples heuristically, here by the presence of a _fields attribute.
if (PyType_FastSubclass(reinterpret_cast<PyTypeObject*>(type.ptr()), Py_TPFLAGS_TUPLE_SUBCLASS))
[[unlikely]] {
if (PyObject* _fields = PyObject_GetAttrString(type.ptr(), "_fields")) [[unlikely]] {
if (PyObject* _fields = PyObject_GetAttr(type.ptr(), Py_Get_ID(_fields))) [[unlikely]] {
bool fields_ok = static_cast<bool>(PyTuple_CheckExact(_fields));
if (fields_ok) [[likely]] {
for (const auto& field : py::reinterpret_borrow<py::tuple>(_fields)) {
Expand All @@ -345,8 +369,8 @@ inline bool IsNamedTupleClassImpl(const py::handle& type) {
Py_DECREF(_fields);
if (fields_ok) [[likely]] {
// NOLINTNEXTLINE[readability-use-anyofallof]
for (const char* name : {"_make", "_asdict"}) {
if (PyObject* attr = PyObject_GetAttrString(type.ptr(), name)) [[likely]] {
for (PyObject* name : {Py_Get_ID(_make), Py_Get_ID(_asdict)}) {
if (PyObject* attr = PyObject_GetAttr(type.ptr(), name)) [[likely]] {
bool result = static_cast<bool>(PyCallable_Check(attr));
Py_DECREF(attr);
if (!result) [[unlikely]] {
Expand Down Expand Up @@ -396,7 +420,7 @@ inline py::tuple NamedTupleGetFields(const py::handle& object) {
static_cast<std::string>(py::repr(object)) + ".");
}
}
return py::getattr(type, "_fields");
return py::getattr(type, Py_Get_ID(_fields));
}

inline bool IsStructSequenceClassImpl(const py::handle& type) {
Expand All @@ -409,8 +433,9 @@ inline bool IsStructSequenceClassImpl(const py::handle& type) {
PyTuple_GET_ITEM(type_object->tp_bases, 0) == reinterpret_cast<PyObject*>(&PyTuple_Type) &&
!static_cast<bool>(PyType_HasFeature(type_object, Py_TPFLAGS_BASETYPE))) [[unlikely]] {
// NOLINTNEXTLINE[readability-use-anyofallof]
for (const char* name : {"n_fields", "n_sequence_fields", "n_unnamed_fields"}) {
if (PyObject* attr = PyObject_GetAttrString(type.ptr(), name)) [[unlikely]] {
for (PyObject* name :
{Py_Get_ID(n_fields), Py_Get_ID(n_sequence_fields), Py_Get_ID(n_unnamed_fields)}) {
if (PyObject* attr = PyObject_GetAttr(type.ptr(), name)) [[unlikely]] {
bool result = static_cast<bool>(PyLong_CheckExact(attr));
Py_DECREF(attr);
if (!result) [[unlikely]] {
Expand Down Expand Up @@ -457,7 +482,7 @@ inline py::tuple StructSequenceGetFields(const py::handle& object) {
}
}

const auto n_sequence_fields = getattr(type, "n_sequence_fields").cast<ssize_t>();
const auto n_sequence_fields = getattr(type, Py_Get_ID(n_sequence_fields)).cast<ssize_t>();
auto* members = reinterpret_cast<PyTypeObject*>(type.ptr())->tp_members;
py::tuple fields{n_sequence_fields};
for (ssize_t i = 0; i < n_sequence_fields; ++i) {
Expand All @@ -481,13 +506,14 @@ inline void TotalOrderSort(py::list& list) { // NOLINT[runtime/references]
// Sort with `(f'{o.__class__.__module__}.{o.__class__.__qualname__}', o)`
auto sort_key_fn = py::cpp_function([](const py::object& o) {
py::handle t = py::type::handle_of(o);
py::str qualname{
static_cast<std::string>(py::getattr(t, "__module__").cast<py::str>()) +
"." +
static_cast<std::string>(py::getattr(t, "__qualname__").cast<py::str>())};
py::str qualname{static_cast<std::string>(
py::getattr(t, Py_Get_ID(__module__)).cast<py::str>()) +
"." +
static_cast<std::string>(
py::getattr(t, Py_Get_ID(__qualname__)).cast<py::str>())};
return py::make_tuple(qualname, o);
});
list.attr("sort")(py::arg("key") = sort_key_fn);
py::getattr(list, Py_Get_ID(sort))(py::arg("key") = sort_key_fn);
} catch (py::error_already_set& ex2) {
if (ex2.matches(PyExc_TypeError)) [[likely]] {
// Found incomparable user-defined key types.
Expand Down
16 changes: 8 additions & 8 deletions src/treespec/flatten.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,16 @@ bool PyTreeSpec::FlattenIntoImpl(const py::handle& handle,
auto dict = py::reinterpret_borrow<py::dict>(handle);
py::list keys = DictKeys(dict);
if (node.kind != PyTreeKind::OrderedDict) [[likely]] {
node.ordered_keys = keys.attr("copy")();
node.ordered_keys = py::getattr(keys, Py_Get_ID(copy))();
TotalOrderSort(keys);
}
for (const py::handle& key : keys) {
recurse(dict[key]);
}
node.arity = GET_SIZE<py::dict>(dict);
if (node.kind == PyTreeKind::DefaultDict) [[unlikely]] {
node.node_data =
py::make_tuple(py::getattr(handle, "default_factory"), std::move(keys));
node.node_data = py::make_tuple(py::getattr(handle, Py_Get_ID(default_factory)),
std::move(keys));
} else [[likely]] {
node.node_data = std::move(keys);
}
Expand All @@ -125,7 +125,7 @@ bool PyTreeSpec::FlattenIntoImpl(const py::handle& handle,
case PyTreeKind::Deque: {
auto list = handle.cast<py::list>();
node.arity = GET_SIZE<py::list>(list);
node.node_data = py::getattr(handle, "maxlen");
node.node_data = py::getattr(handle, Py_Get_ID(maxlen));
for (ssize_t i = 0; i < node.arity; ++i) {
recurse(GET_ITEM_HANDLE<py::list>(list, i));
}
Expand Down Expand Up @@ -289,16 +289,16 @@ bool PyTreeSpec::FlattenIntoWithPathImpl(const py::handle& handle,
auto dict = py::reinterpret_borrow<py::dict>(handle);
py::list keys = DictKeys(dict);
if (node.kind != PyTreeKind::OrderedDict) [[likely]] {
node.ordered_keys = keys.attr("copy")();
node.ordered_keys = py::getattr(keys, Py_Get_ID(copy))();
TotalOrderSort(keys);
}
for (const py::handle& key : keys) {
recurse(dict[key], key);
}
node.arity = GET_SIZE<py::dict>(dict);
if (node.kind == PyTreeKind::DefaultDict) [[unlikely]] {
node.node_data =
py::make_tuple(py::getattr(handle, "default_factory"), std::move(keys));
node.node_data = py::make_tuple(py::getattr(handle, Py_Get_ID(default_factory)),
std::move(keys));
} else [[likely]] {
node.node_data = std::move(keys);
}
Expand All @@ -319,7 +319,7 @@ bool PyTreeSpec::FlattenIntoWithPathImpl(const py::handle& handle,
case PyTreeKind::Deque: {
auto list = handle.cast<py::list>();
node.arity = GET_SIZE<py::list>(list);
node.node_data = py::getattr(handle, "maxlen");
node.node_data = py::getattr(handle, Py_Get_ID(maxlen));
for (ssize_t i = 0; i < node.arity; ++i) {
recurse(GET_ITEM_HANDLE<py::list>(list, i), py::int_(i));
}
Expand Down
10 changes: 6 additions & 4 deletions src/treespec/treespec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1162,11 +1162,13 @@ std::string PyTreeSpec::ToStringImpl() const {

case PyTreeKind::NamedTuple: {
py::object type = node.node_data;
auto fields = py::reinterpret_borrow<py::tuple>(py::getattr(type, "_fields"));
auto fields =
py::reinterpret_borrow<py::tuple>(py::getattr(type, Py_Get_ID(_fields)));
EXPECT_EQ(GET_SIZE<py::tuple>(fields),
node.arity,
"Number of fields and entries does not match.");
std::string kind = static_cast<std::string>(py::str(py::getattr(type, "__name__")));
std::string kind =
static_cast<std::string>(py::str(py::getattr(type, Py_Get_ID(__name__))));
sstream << kind << "(";
bool first = true;
auto child_iter = agenda.end() - node.arity;
Expand Down Expand Up @@ -1257,8 +1259,8 @@ std::string PyTreeSpec::ToStringImpl() const {
}

case PyTreeKind::Custom: {
std::string kind =
static_cast<std::string>(py::str(py::getattr(node.custom->type, "__name__")));
std::string kind = static_cast<std::string>(
py::str(py::getattr(node.custom->type, Py_Get_ID(__name__))));
sstream << "CustomTreeNode(" << kind << "[";
if (node.node_data) [[likely]] {
sstream << static_cast<std::string>(py::repr(node.node_data));
Expand Down
Loading