gh-141968: Use take_bytes to remove copy in _pyio.BytesIO.read()#149850
Conversation
| b = self._buffer[self._pos : newpos] | ||
| self._pos = newpos | ||
| return bytes(b) | ||
| return b.take_bytes() |
There was a problem hiding this comment.
As far as I can tell b = self._buffer[self._pos : newpos] already copies the bytearray so we can safely use take_bytes()
d67f989 to
d32d3b0
Compare
|
cc @cmaloney |
There was a problem hiding this comment.
I think your analysis is correct / the slice already does a copy so take_bytes() is safe in this case.
I've been experimenting to see if there is a way to avoid the temporary bytearray / go straight from the original to final bytes but haven't found anything simpler so far (would save allocating the bytearray head). Can swap in a memoryview but I don't find that simpler, just a different shape.
Could you remove the news file? I added skip news so PR checks should pass. This is just an optimizing refactor and _pyio is not widely used so people are unlikely to see an impact from this. End users only see the C _io implementation. The CPython test suite will run a little faster though which is nice :)
also: How did you spot/find this one? (Wondering if there's more tips and tricks should add to the whatsnew)
|
Please don't force push to CPython PRs: https://devguide.python.org/getting-started/pull-request-lifecycle/index.html#don-t-force-push |
Done. I was just trying to make CI happy :)
Honestly, I'm not 100% sure anymore. I stumbled upon #149852 by accident some time ago and before making the PR had a quick look at places where |
Sorry, will keep in mind for the future. Just force pushed to remove the last two commits which added the news entry. |
|
|
CI failure unrelated to this change |
This removes a copy going from
bytearraytobytesin_pyio.BytesIO.read().