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-129005: Move bytearray to use bytes as a buffer by cmaloney · Pull Request #130563 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Feb 26, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  1. Add tp_new to guarantee ob_bytes_head is always set, often to the empty bytes singleton.
  2. ob_alloc information is now redundant, added assertion to validate that, would it make sense to deprecate?
  3. There's a lot of bytearray code very similar to bytes code, more could likely be just proxied to the bytes now (Ex. construction from an object in __new__). Here just focusing on the swap as that enables optimizations.
  4. If bytearray is passed a single-reference bytes it could potentially take "ownership" of it without copying the bytes, for now not implemented.

This enables adding bytearray._detach() which I plan to do in a separate PR.

TODO

  • Single byte strings passed to PyBytes_FromStringAndSize returns immortal objects that shouldn't be mutated. Not sure how to best work around that. _io.FileIO.readall() can't run into that case as it never passes in a string to start with, just a size.

Performance

# _io.FileIO.readall of a large file
./python -m test -M8g -uall test_largefile -m test.test_largefile.CLargeFileTest.test_large_read

# _pyio.FileIO.readall of a large file
./python -m test -M8g -uall test_largefile -m test.test_largefile.PyLargeFileTest.test_large_read

On my machine (AMD 64 bit Linux, Optimized build):
_io takes: ~0.791s and uses ~2GB of RAM
_pyio current: ~1.073s and uses ~4GB of RAM
_pyio with bytearray._detach: ~0.887s and uses ~2GB of RAM

Perf checking no major swings in an optimized build: ./python -E -bb -Wd -m test -uall -M32G test_bytes test_capi.test_bytearray -vvv

before: ~1.4s
after: ~1.4s

Previous discussion: https://discuss.python.org/t/add-zero-copy-conversion-of-bytearray-to-bytes-by-providing-bytes/79164

cc: @serhiy-storchaka @vstinner

  • Issue: Reduce copies when reading files in pyio, match behavior of _io #129005

Sorry, something went wrong.

Uh oh!

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

All reactions
@cmaloney
pythongh-129005: Move bytearray to use bytes as a buffer
d85f2ea
1. add `tp_new` to guarantee `ob_bytes_head` is always set, often to the empty
   bytes singleton.
2. `ob_alloc` information is now redundant, added assertion to validate that,
   would it make sense to deprecate?
3. There's a lot of `bytearray` code very similar to `bytes` code, more could
   likely be just proxied to the `bytes` now. Here just focusing on the swap as
   that enables optimizations.
4. If `bytearray` is passed a single-reference bytes it could potentially take
   "ownership" of it without copying the bytes, for now not implemented.

This enables adding `bytearray._detach()` which I plan to do in a separate PR.

```bash
 # `_io` large file read test
./python -m test -M8g -uall test_largefile -m test.test_largefile.CLargeFileTest.test_large_read

 # `_pyio` large file read test
./python -m test -M8g -uall test_largefile -m test.test_largefile.PyLargeFileTest.test_large_read
```

On my machine (AMD 64 bit Linux, Optimized build):
`_io` takes: ~0.791s and uses ~2GB of RAM
`_pyio` current: ~1.073s and uses ~4GB of RAM
`_pyio` w/ bytearray._detach: ~0.887s and uses ~2GB of RAM

Perf checking no major swings in an optimized build:
`./python -E -bb -Wd -m test -uall -M32G test_bytes test_capi.test_bytearray -vvv`

before: ~1.4s
after: ~1.5s

Previous discussion: https://discuss.python.org/t/add-zero-copy-conversion-of-bytearray-to-bytes-by-providing-bytes/79164
@bedevere-app bedevere-app Bot added the awaiting review label Feb 26, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Feb 26, 2025
Reduce copies when reading files in pyio, match behavior of _io #129005
Closed
@cmaloney
Update 2025-02-25-20-31-36.gh-issue-129005.0KBps9.rst
5c7922c
cmaloney
cmaloney commented Feb 27, 2025
View reviewed changes
Comment thread Objects/bytearrayobject.c
}

/* Too small, grow allocation */
if (_PyBytes_Resize(&obj->ob_bytes_head, alloc - 1) < 0) {

@cmaloney cmaloney Feb 27, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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.

behavior change: _PyBytes_Resize behaves differently than PyMem_Realloc here. _PyBytes_Resize can change the location of the buffer. That means the address returned from PyByteArray_AS_STRING may change on any resize. Previously it changed only when went from empty <-> non-empty bytearray (empty points to a constant storage).

https://docs.python.org/3/c-api/memory.html#c.PyMem_Realloc

Sorry, something went wrong.

Uh oh!

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

All reactions
@cmaloney

cmaloney commented Mar 12, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Closing while discussion is ongoing, if decide to ship, will make a new github issue for the new feature this implements.

All reactions

Sorry, something went wrong.

Uh oh!

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

@cmaloney cmaloney closed this Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review

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.

1 participant

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