Skip to content

Commit 5d35b49

Browse files
swolchokpytorchmergebot
authored andcommitted
Fix forced copying def_property_readonly for FunctionSchema & friends (pytorch#161301)
This took me a bit to figure out and I'm pretty sure I've looked at this code before. Pybind uses `return_value_policy::reference_internal` for `def_property`, which [causes the owning object to be kept alive for the lifespan of the return value](https://pybind11.readthedocs.io/en/stable/advanced/functions.html), allowing the getter to safely avoid copying the property value. However, lambdas act like they return `auto`, not `decltype(auto)`, so our lambdas themselves were forcing copies! Testing: observed std::vector<Argument> copying disappear in Linux perf profile of someOpInfo._schema.arguments/returns (in _python_dispatch.correct_storage_aliasing). Pull Request resolved: pytorch#161301 Approved by: https://github.com/Skylion007, https://github.com/malfet, https://github.com/wconstab
1 parent db62284 commit 5d35b49

File tree

1 file changed

+10
-16
lines changed

1 file changed

+10
-16
lines changed

torch/csrc/jit/python/init.cpp

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1958,18 +1958,15 @@ void initJITBindings(PyObject* module) {
19581958
std::vector<Argument>,
19591959
bool,
19601960
bool>())
1961-
.def_property_readonly(
1962-
"name", [](const FunctionSchema& self) { return self.name(); })
1963-
.def_property_readonly(
1964-
"overload_name",
1965-
[](const FunctionSchema& self) { return self.overload_name(); })
1966-
.def_property_readonly(
1967-
"arguments",
1968-
[](const FunctionSchema& self) { return self.arguments(); })
1969-
.def_property_readonly(
1970-
"returns", [](const FunctionSchema& self) { return self.returns(); })
1961+
.def_property_readonly("name", &FunctionSchema::name)
1962+
.def_property_readonly("overload_name", &FunctionSchema::overload_name)
1963+
.def_property_readonly("arguments", &FunctionSchema::arguments)
1964+
.def_property_readonly("returns", &FunctionSchema::returns)
19711965
.def(
19721966
"is_backward_compatible_with",
1967+
// FunctionSchema::isBackwardCompatibleWith has an extra
1968+
// defaulted argument, so we can't just use a
1969+
// pointer-to-member here.
19731970
[](const FunctionSchema& self, const FunctionSchema& old_schema) {
19741971
return self.isBackwardCompatibleWith(old_schema);
19751972
})
@@ -2024,12 +2021,9 @@ void initJITBindings(PyObject* module) {
20242021
std::optional<IValue>,
20252022
bool,
20262023
std::optional<AliasInfo>>())
2027-
.def_property_readonly(
2028-
"name", [](const Argument& self) { return self.name(); })
2029-
.def_property_readonly(
2030-
"type", [](const Argument& self) { return self.type(); })
2031-
.def_property_readonly(
2032-
"real_type", [](const Argument& self) { return self.real_type(); })
2024+
.def_property_readonly("name", &Argument::name)
2025+
.def_property_readonly("type", &Argument::type)
2026+
.def_property_readonly("real_type", &Argument::real_type)
20332027
.def_property_readonly(
20342028
"N",
20352029
[](const Argument& self) -> py::object {

0 commit comments

Comments
 (0)