classification
Title: PyBytes_Concat could try to concat in-place
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, nikratio, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2014-04-28 19:47 by pitrou, last changed 2014-05-06 16:43 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue_21377.diff nikratio, 2014-04-29 04:01 review
issue_21377_r2.diff nikratio, 2014-04-29 04:17 review
issue_21377_r3.diff nikratio, 2014-04-30 01:44 review
issue_21377_r4.diff nikratio, 2014-05-01 01:23 review
Messages (8)
msg217406 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 19:47
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)
msg217462 - (view) Author: Nikolaus Rath (nikratio) * Date: 2014-04-29 04:01
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?
msg217480 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 07:55
> Tentative patch attached. The test suite still passes, but I'm not
> sure if it actually exerts the new code path.

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.
msg217498 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-04-29 08:45
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?
msg217499 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 08:46
> 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?

The latter, IMO. This issue is about the C API _PyBytes_Concat.
msg217694 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-05-01 12:26
Thanks! The latest patch looks fine to me.
msg217695 - (view) Author: Roundup Robot (python-dev) Date: 2014-05-01 12:36
New changeset 4ed1b6c7e2f3 by Antoine Pitrou in branch 'default':
Issue #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
msg217994 - (view) Author: Roundup Robot (python-dev) Date: 2014-05-06 16:43
New changeset 6234f4caba57 by Zachary Ware in branch 'default':
Issue #21442: Fix MSVC compiler warning introduced by issue21377.
http://hg.python.org/cpython/rev/6234f4caba57
History
Date User Action Args
2014-05-06 16:43:11python-devsetmessages: + msg217994
2014-05-01 12:43:11pitrousetstatus: open -> closed
resolution: fixed
stage: resolved
2014-05-01 12:36:31python-devsetnosy: + python-dev
messages: + msg217695
2014-05-01 12:26:34pitrousetmessages: + msg217694
2014-05-01 01:23:02nikratiosetfiles: + issue_21377_r4.diff
2014-04-30 01:44:30nikratiosetfiles: + issue_21377_r3.diff
2014-04-29 08:46:06pitrousetmessages: + msg217499
2014-04-29 08:45:21hayposetmessages: + msg217498
2014-04-29 07:55:59pitrousetmessages: + msg217480
2014-04-29 04:17:21nikratiosetfiles: + issue_21377_r2.diff
2014-04-29 04:01:01nikratiosetfiles: + issue_21377.diff
keywords: + patch
messages: + msg217462
2014-04-28 19:47:24pitroucreate