GeezerVison is showing github.com — gh-139871: Optimize small takes in bytearray.take_bytes by cmaloney · Pull Request #141741 · python/cpython
Conversation
cmaloney
commented
Nov 19, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
When less than half the buffer is taken just copy that small part out rather than doing a big alloc + memcpy + big shrink.
cc: @vstinner, @encukou
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
When less than half the buffer is taken just copy that small part out
rather than doing a big alloc + memmove + big shrink.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Hmm, would it make sense to take this path on (self->ob_start != self->ob_bytes), too? In that case the code currently goes on to memmove data.
OTOH, doing this would cause the bytearray into using this path in future takes, until a reallocation happens...
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cmaloney
commented
Nov 20, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Hmm, would it make sense to take this path on (self->ob_start != self->ob_bytes), too?
I'd need to measure more / not sure. The realloc code does memcpy + malloc if 25% of space can be saved:
|
if (4 * nbytes > 3 * size) { |
Doing a malloc + memcpy + free instead of the memmove + malloc + memcpy + free would be good but not sure how much to tune to the particular allocator constants.
My theory for usage pattern has been reading a block of bytes off a stream (network or terminal), appending (or readinto) the bytearray, then taking little chunks out of that as bytes. With that bytearray realloc code should recompact periodically effectively meaning this turns into a reasonably efficient ringbuffer.... (for that use case a "please repack but don't reduce capacity flag may be useful"). Not sure how common that usage is going to be though
|
encukou reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
That sounds like readlines().
Anyway, no need to block this PR on that. Thank you!
|
cmaloney reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit e265ce8.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/345/builds/12729) and take a look at the build logs.
- Check if the failure is related to this commit (e265ce8) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/#/builders/345/builds/12729
Failed tests:
Failed subtests:
- test_timerfd_interval - test.test_os.test_os.TimerfdTests.test_timerfd_interval
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_os/test_os.py", line 4009, in test_timerfd_interval
self.assertEqual(self.read_count_signaled(fd), 1)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 2 != 1
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_os/test_os.py", line 4017, in test_timerfd_interval
self.assertEqual(self.read_count_signaled(fd), count)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 4 != 3
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Test failure looks independent of this change, os_read_impl doesn't use bytearray and this doesn't touch the other part of it int.from_bytes (in general things don't use .take_bytes very much yet)
|
encukou reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
StanFromIreland
pushed a commit
to StanFromIreland/cpython
that referenced
this pull request
Dec 6, 2025
…GH-141741)
When less than half the buffer is taken just copy that small part out
rather than doing a big alloc + memmove + big shrink.
ashm-dev
pushed a commit
to ashm-dev/cpython
that referenced
this pull request
Dec 8, 2025
…GH-141741)
When less than half the buffer is taken just copy that small part out
rather than doing a big alloc + memmove + big shrink.
Reviewers
encukou
encukou left review comments
vstinner
vstinner approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|