GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-139871: Optimize small takes in bytearray.take_bytes by cmaloney · Pull Request #141741 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Nov 19, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

  • Issue: Add .take_bytes([n]) a zero-copy path from bytearray to bytes #139871

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@cmaloney
pythongh-139871: Optimize small takes in bytearray.take_bytes
783717c
When less than half the buffer is taken just copy that small part out
rather than doing a big alloc + memmove + big shrink.
@bedevere-app bedevere-app Bot added the awaiting review label Nov 19, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Nov 19, 2025
Add .take_bytes([n]) a zero-copy path from bytearray to bytes #139871
Closed
@cmaloney cmaloney added the skip news label Nov 19, 2025
encukou
encukou reviewed Nov 19, 2025
View reviewed changes
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Nov 19, 2025
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Nov 19, 2025
@encukou

encukou commented Nov 19, 2025

Copy link
Copy Markdown
Member

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...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cmaloney and others added 2 commits November 19, 2025 12:02
@cmaloney @vstinner
Update Objects/bytearrayobject.c
16a7378
Co-authored-by: Victor Stinner <[email protected]>
@cmaloney @encukou
Update Lib/test/test_bytes.py
25a03d9
Co-authored-by: Petr Viktorin <[email protected]>
@cmaloney

cmaloney commented Nov 20, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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:

cpython/Objects/obmalloc.c

Line 2676 in ca1e86f

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Nov 20, 2025

Copy link
Copy Markdown
Member

That sounds like readlines().

Anyway, no need to block this PR on that. Thank you!

cmaloney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
encukou merged commit e265ce8 into python:main Nov 20, 2025
44 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 20, 2025
@bedevere-bot

bedevere-bot commented Nov 20, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit e265ce8.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (e265ce8) or if it is a false positive.
  5. 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:

  • test.test_os.test_os

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cmaloney
cmaloney deleted the ba_tb_keep_majority branch November 20, 2025 08:13
@cmaloney

cmaloney commented Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

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
All reactions
  • 👍 1 reaction

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
@cmaloney @StanFromIreland
pythongh-139871: Optimize small takes in bytearray.take_bytes (python…
5f6b0c3
…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
@cmaloney @ashm-dev
pythongh-139871: Optimize small takes in bytearray.take_bytes (python…
1b0326a
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments
@vstinner vstinner vstinner approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

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.

4 participants

@cmaloney @encukou @bedevere-bot @vstinner
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.