Repository navigation
PyBytes_Concat could try to concat in-place #65576
Description
| BPO | 21377 |
|---|---|
| Nosy | @pitrou, @vstinner |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2014-05-01.12:43:11.785>
created_at = <Date 2014-04-28.19:47:24.270>
labels = ['interpreter-core', 'performance']
title = 'PyBytes_Concat could try to concat in-place'
updated_at = <Date 2014-05-06.16:43:11.936>
user = 'https://github.com/pitrou'bugs.python.org fields:
activity = <Date 2014-05-06.16:43:11.936>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2014-05-01.12:43:11.785>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2014-04-28.19:47:24.270>
creator = 'pitrou'
dependencies = []
files = ['35084', '35085', '35100', '35123']
hgrepos = []
issue_num = 21377
keywords = ['patch']
message_count = 8.0
messages = ['217406', '217462', '217480', '217498', '217499', '217694', '217695', '217994']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'nikratio', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue21377'
versions = ['Python 3.5']
Activity
pitrou commentedon Apr 28, 2014
Currently, PyBytes_Concat always creates a new bytes object for the result. However, when Py_REFCNT(*pv) == 1, it could instead call _PyBytes_Resize() and then concat the second argument in place.
(like e.g. _PyUnicode_Append does)
nikratio commentedon Apr 29, 2014
Tentative patch attached. The test suite still passes, but I'm not sure if it actually exerts the new code path. Is there a standard way to test the C api?
pitrou commentedon Apr 29, 2014
A quick grep shows me that it should be exercised at least by Modules/_io/bufferedio.c.
Otherwise, the way to test the C API is to add a function to Modules/_testcapi.c, and test that function from the relevant test file.
vstinner commentedon Apr 29, 2014
If I remember correctly, ceval.c has an optmisation for str += str even if
the refcount is 2. Do we need to implement it or suggest to use bytearray
or b''.join() instead?
pitrou commentedon Apr 29, 2014
The latter, IMO. This issue is about the C API _PyBytes_Concat.
pitrou commentedon May 1, 2014
Thanks! The latest patch looks fine to me.
python-dev commentedon May 1, 2014
New changeset 4ed1b6c7e2f3 by Antoine Pitrou in branch 'default':
Issue bpo-21377: PyBytes_Concat() now tries to concatenate in-place when the first argument has a reference count of 1.
http://hg.python.org/cpython/rev/4ed1b6c7e2f3
python-dev commentedon May 6, 2014
New changeset 6234f4caba57 by Zachary Ware in branch 'default':
Issue bpo-21442: Fix MSVC compiler warning introduced by bpo-21377.
http://hg.python.org/cpython/rev/6234f4caba57