Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-41486: Add _BlocksOutputBuffer for bz2/lzma/zlib modules #21740

Merged
merged 18 commits into from Apr 28, 2021

Conversation

animalize
Copy link
Contributor

@animalize animalize commented Aug 5, 2020

And add .readall() function to _compression.DecompressReader class.

Oct 16, 2020 update: No compiler warnings.

https://bugs.python.org/issue41486

@animalize animalize force-pushed the blocks_output_buffer branch 2 times, most recently from 050893a to 57d1666 Compare Aug 6, 2020
@animalize animalize force-pushed the blocks_output_buffer branch 3 times, most recently from 3fcc50f to bd3e17f Compare Sep 8, 2020
@animalize animalize force-pushed the blocks_output_buffer branch 14 times, most recently from 36b76f5 to c3b4c46 Compare Oct 23, 2020
@taleinat taleinat added the performance Performance or resource usage label Oct 25, 2020
Modules/blocks_output_buffer.h Outdated Show resolved Hide resolved
Lib/_compression.py Outdated Show resolved Hide resolved
@animalize animalize force-pushed the blocks_output_buffer branch 5 times, most recently from 3b615af to 7b71b40 Compare Dec 14, 2020
@bedevere-bot
Copy link

bedevere-bot commented Apr 12, 2021

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

@animalize
Copy link
Contributor Author

animalize commented Apr 12, 2021

This commit Merge branch 'master' into blocks_output_buffer solved code conflict in Doc/whatsnew/3.10.rst, so it's empty.

@animalize animalize force-pushed the blocks_output_buffer branch 2 times, most recently from 01e9b41 to 55705f6 Compare Apr 27, 2021
Copy link
Member

@gpshead gpshead left a comment

great code! one unused bit of code to delete and a few assertions on constants that i think can be compile time errors instead?

this is ready to go in after that.

Include/internal/pycore_blocks_output_buffer.h Outdated Show resolved Hide resolved
Modules/_bz2module.c Outdated Show resolved Hide resolved
Modules/_lzmamodule.c Outdated Show resolved Hide resolved
Modules/zlibmodule.c Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Apr 28, 2021

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@animalize
Copy link
Contributor Author

animalize commented Apr 28, 2021

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Apr 28, 2021

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

@gpshead gpshead merged commit f9bedb6 into python:master Apr 28, 2021
12 checks passed
@gpshead gpshead self-assigned this Apr 28, 2021
@gpshead gpshead added the type-feature A feature request or enhancement label Apr 28, 2021
@animalize animalize deleted the blocks_output_buffer branch May 1, 2021
kreathon pushed a commit to kreathon/cpython that referenced this pull request May 2, 2021
)

Faster bz2/lzma/zlib via new output buffering.
Also adds .readall() function to _compression.DecompressReader class
to take best advantage of this in the consume-all-output at once scenario.

Often a 5-20% speedup in common scenarios due to less data copying.

Contributed by Ma Lin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants