Skip to content

bpo-47053: Refactor BINARY_OP_INPLACE_ADD_UNICODE#32122

Merged
sweeneyde merged 2 commits intopython:mainfrom
sweeneyde:inplace_add_unicode_refactor
Mar 29, 2022
Merged

bpo-47053: Refactor BINARY_OP_INPLACE_ADD_UNICODE#32122
sweeneyde merged 2 commits intopython:mainfrom
sweeneyde:inplace_add_unicode_refactor

Conversation

@sweeneyde
Copy link
Member

@sweeneyde sweeneyde commented Mar 26, 2022

Letting PyUnicode_Append directly modify the target local avoids the next STORE_FAST, and it avoids the GETLOCAL(next_oparg) = NULL;.

https://bugs.python.org/issue47053

@sweeneyde
Copy link
Member Author

sweeneyde commented Mar 26, 2022

Microbenchmark:

./python -m pyperf timeit -s "r = 'a'*10_000; s=''" "for c in r: s += c"

Mean +- std dev: [before] 454 us +- 3 us -> [after] 437 us +- 4 us: 1.04x faster

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

LGTM.

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()

@sweeneyde sweeneyde merged commit 7881549 into python:main Mar 29, 2022
@sweeneyde sweeneyde deleted the inplace_add_unicode_refactor branch March 29, 2022 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants