gh-145860: Eliminate unnecessary refcount operations in BUILD_INTERPOLATION and BUILD_TEMPLATE#148201
gh-145860: Eliminate unnecessary refcount operations in BUILD_INTERPOLATION and BUILD_TEMPLATE#148201Siyet wants to merge 2 commits into
Conversation
…ATION/BUILD_TEMPLATE Make _PyInterpolation_Build and _PyTemplate_Build steal references to their arguments instead of borrowing and incref'ing them. This avoids redundant incref/decref pairs in the bytecode handlers, matching the pattern already used by all other BUILD instructions.
Add comments to _PyInterpolation_Build and _PyTemplate_Build declarations in pycore_interpolation.h and pycore_template.h.
|
This PR is stale because it has been open for 30 days with no activity. |
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
Looks good overall.
Can you rename the two functions, adding a "Steal" suffix?
| #define _PyInterpolation_CheckExact(op) Py_IS_TYPE((op), &_PyInterpolation_Type) | ||
|
|
||
| // Steals references to value, str, and format_spec (even on failure). | ||
| PyAPI_FUNC(PyObject *) _PyInterpolation_Build(PyObject *value, PyObject *str, |
There was a problem hiding this comment.
| PyAPI_FUNC(PyObject *) _PyInterpolation_Build(PyObject *value, PyObject *str, | |
| PyAPI_FUNC(PyObject *) _PyInterpolation_BuildSteal(PyObject *value, PyObject *str, |
If a function steals references, we usually add the "Steal" suffix to the name, to make that clear.
| extern PyObject *_PyTemplate_Concat(PyObject *self, PyObject *other); | ||
|
|
||
| // Steals references to strings and interpolations (even on failure). | ||
| PyAPI_FUNC(PyObject *) _PyTemplate_Build(PyObject *strings, PyObject *interpolations); |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
2 participants
BUILD_INTERPOLATIONandBUILD_TEMPLATEcalled_PyInterpolation_Buildand_PyTemplate_Buildrespectively, which incremented the reference counts oftheir arguments via
Py_NewRef. The bytecode handlers then immediatelydecremented those same references via
PyStackRef_CLOSE. This incref/decrefpair is redundnat.
All other
BUILDinstructions (BUILD_TUPLE,BUILD_LIST,BUILD_SET,BUILD_MAP) already avoid this overhead by using "steal" semantics, where thecalled function takes ownership of the references directly.
This change makes
_PyInterpolation_Buildand_PyTemplate_Buildstealreferences to their arguments (even on failure, handling cleanup internally),
and updates the bytecoed handlers to pass owned references via
PyStackRef_AsPyObjectStealinstead of borrowing and closing. The two internalcallers of
_PyTemplate_Buildintemplateobject.c(template_new,template_concat_templates) are updated accordingly.This is a pure optimization with no semantic change. Reduces the number of
atomic refcount operations per
BUILD_INTERPOLATIONby 6 (3 incref + 3 decref)and per
BUILD_TEMPLATEby 4 (2 incref + 2 decref).BUILD_INTERPOLATIONandBUILD_TEMPLATEperform unnecessary refcount operations #145860