Skip to content

bpo-47053: Refactor BINARY_OP_INPLACE_ADD_UNICODE #32122

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

Merged
merged 2 commits into from
Mar 29, 2022
Merged

bpo-47053: Refactor BINARY_OP_INPLACE_ADD_UNICODE #32122

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56427c3
Refactor BINARY_OP_INPLACE_ADD_UNICODE
sweeneyde Mar 26, 2022
2fc50ee
Merge branch 'main' into inplace_add_unicode_refactor
sweeneyde Mar 29, 2022
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
32 changes: 18 additions & 14 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2003,28 +2003,32 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
DEOPT_IF(!PyUnicode_CheckExact(left), BINARY_OP);
DEOPT_IF(Py_TYPE(right) != Py_TYPE(left), BINARY_OP);
_Py_CODEUNIT true_next = next_instr[INLINE_CACHE_ENTRIES_BINARY_OP];
int next_oparg = _Py_OPARG(true_next);
assert(_Py_OPCODE(true_next) == STORE_FAST ||
_Py_OPCODE(true_next) == STORE_FAST__LOAD_FAST);
/* In the common case, there are 2 references to the value
* stored in 'variable' when the v = v + ... is performed: one
* on the value stack (in 'v') and one still stored in the
* 'variable'. We try to delete the variable now to reduce
* the refcnt to 1.
*/
PyObject *var = GETLOCAL(next_oparg);
DEOPT_IF(var != left, BINARY_OP);
PyObject **target_local = &GETLOCAL(_Py_OPARG(true_next));
DEOPT_IF(*target_local != left, BINARY_OP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the last iteration here for example:

>>> pairs = [('foo',) * 2 for _ in range(10_000)]
>>> pairs.append(('foo', 'bar'))
>>> def f():
...     for x, y in pairs:
...         x = y + 'a'
...
>>> f()

STAT_INC(BINARY_OP, hit);
GETLOCAL(next_oparg) = NULL;
/* Handle `left = left + right` or `left += right` for str.
*
* When possible, extend `left` in place rather than
* allocating a new PyUnicodeObject. This attempts to avoid
* quadratic behavior when one neglects to use str.join().
*
* If `left` has only two references remaining (one from
* the stack, one in the locals), DECREFing `left` leaves
* only the locals reference, so PyUnicode_Append knows
* that the string is safe to mutate.
*/
assert(Py_REFCNT(left) >= 2);
Py_DECREF(left); // XXX never need to dealloc
STACK_SHRINK(1);
PyUnicode_Append(&TOP(), right);
STACK_SHRINK(2);
PyUnicode_Append(target_local, right);
Py_DECREF(right);
if (TOP() == NULL) {
if (*target_local == NULL) {
goto error;
}
JUMPBY(INLINE_CACHE_ENTRIES_BINARY_OP);
// The STORE_FAST is already done.
JUMPBY(INLINE_CACHE_ENTRIES_BINARY_OP + 1);
NOTRACE_DISPATCH();
}

Expand Down