GeezerVison

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

GeezerVison is showing github.com — gh-95534: Improve gzip reading speed by 10% by rhpvorderman · Pull Request #97664 · python/cpython

Conversation

rhpvorderman commented Sep 30, 2022 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

For motivation check the github issue. #95534
Performance figures before (the used gzip file is the tar source distribution from github):

./python -m pyperf timeit -s "import gzip; g=gzip.open('cpython-3.10.7.tar.gz', 'rb'); it=iter(lambda:g.read(128*1024), b'');" "for _ in it: pass"
.....................
Mean +- std dev: 301 ms +- 2 ms

after:

./python -m pyperf timeit -s "import gzip; g=gzip.open('cpython-3.10.7.tar.gz', 'rb'); it=iter(lambda:g.read(128*1024), b'');" "for _ in it: pass"
.....................
Mean +- std dev: 270 ms +- 1 ms

Performance tests where run with all optimizations enabled. I found that --enable-optimizations did not influence the result. So it can be verified without all the PGO stuff.

Change summary:

There is now a gzip.READ_BUFFER_SIZE constant that is 128KB. Other programs that read in 128KB chunks: pigz and cat. So this seems best practice among good programs. Also it is faster than 8 kb chunks.
a zlib._ZlibDecompressor was added. This is the _bz2.BZ2Decompressor ported to zlib. Since the zlib.Decompress object is better for in-memory decompression, the _ZlibDecompressor is hidden. It only makes sense in file decompression, and that is already implemented now in the gzip library. No need to bother the users with this.
The ZlibDecompressor uses the older Cpython arrange_output_buffer functions, as those are faster and more appropriate for the use case.
GzipFile.read has been optimized. There is no longer a unconsumed_tail member to write back to padded file. This is instead handled by the ZlibDecompressor itself, which has an internal buffer. _add_read_data has been inlined, as it was just two calls.

EDIT: While I am adding improvements anyway, I figured I could add another one-liner optimization now to the python -m gzip application. That read chunks in io.DEFAULT_BUFFER_SIZE previously, but has been updated now to use READ_BUFFER_SIZE chunks.
Results:
before:

Benchmark 1: cat cpython-3.10.7.tar.gz | ./python -m gzip -d > /dev/null
  Time (mean ± σ):     389.1 ms ±  12.0 ms    [User: 372.7 ms, System: 19.9 ms]
  Range (min … max):   370.9 ms … 410.2 ms    20 runs

After:

Benchmark 1: cat cpython-3.10.7.tar.gz | ./python -m gzip -d > /dev/null
  Time (mean ± σ):     320.5 ms ±  12.1 ms    [User: 306.4 ms, System: 17.6 ms]
  Range (min … max):   300.0 ms … 339.1 ms    20 runs

For comparison: pigz, the fastest zlib utilizing gzip decompressor, on a single thread. (igzip is faster, but utilizes ISA-L).

Benchmark 1: cat cpython-3.10.7.tar.gz | pigz -p 1 -d > /dev/null
  Time (mean ± σ):     293.8 ms ±   8.4 ms    [User: 288.5 ms, System: 17.0 ms]
  Range (min … max):   277.5 ms … 302.7 ms    20 runs

If we take the pure C pigz program as baseline, the amount of python overhead is reduced drastically from 30% to 10%.

Issue: Faster decompression of gzip files #95534

Sorry, something went wrong.

Uh oh!

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

All reactions

rhpvorderman added 12 commits

September 28, 2022 11:34


          Add code from python-isal project

1e13a89


          Reorder code

6a5cdfd


          Add ZlibDecompressor

809ad5f


          Add zlibdecompressor object

03254b8


          Fix compile warnings

669848a


          Do not use class input

69ff613


          Fix lock stuff

6fa43ae


          Fix incorrect error handling

cdc5972


          Rework _GzipReader to be more efficient


          Properly initialize zstate

6f8b64a


          Add blurb for increased gzip read speed

3e2a4f5


          Make sure self->initialised is set to 0. Reword some comments.

070df1c

bedevere-bot added the awaiting review label

Sep 30, 2022

rhpvorderman added 2 commits

September 30, 2022 09:54


          Add appropriate doctype in blurb

70b7d4d


          Merge branch 'main' into pythongh-95534

22d3893

rhpvorderman commented Sep 30, 2022

Copy link

Copy Markdown

Contributor Author

Once all the reviewing is done, I will squash the commits.

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner commented Sep 30, 2022

Copy link

Copy Markdown

Member

Sorry I'm not a gzip expert so I can't review this. However I'd just like to say that you don't need to squash the commits, the core dev will squash them for you when merging!

Thanks for the thorough research and a solution to speeding up gzip!

All reactions

Sorry, something went wrong.

Uh oh!

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

rhpvorderman commented Sep 30, 2022

Copy link

Copy Markdown

Contributor Author

However I'd just like to say that you don't need to squash the commits, the core dev will squash them for you when merging!

Thank you. It is good to know that "fix lock stuff" and "reorder code" will not get added to the list of commit messages.

Sorry I'm not a gzip expert so I can't review this.

Can you help me with the adress sanitizer? It is not happy about the way I added a new heap type, even though I seem to have done exactly the same thing as for the other heap types in the module. I am at a loss here. It seems to crash in state->ZlibDecompressorType = (PyTypeObject *)PyType_FromModuleAndSpec( . In python-isal I simply use static types as it is much easier, but since both the zlib and bz2 module are all heap types now I figure there must be some important advantage.

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner reviewed

Sep 30, 2022

View reviewed changes

Comment thread

Modules/zlibmodule.c Resolved

Uh oh!

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

rhpvorderman added 5 commits

September 30, 2022 11:40


          Add missing NULL member to ZlibDecompressor_Members

18a7692


          Merge branch 'pythongh-95534' of github.com:rhpvorderman/cpython into p…

d54c8b5

…ythongh-95534


          Remove double comment

c90096f


          Use READ_BUFFER_SIZE in python -m gzip command line application

1c15839


          Fix error in news entry

d0ff4f0

Fidget-Spinner added the performance Performance or resource usage label

Sep 30, 2022


          minor edit, use +=

afd92ab

gpshead requested changes

Sep 30, 2022

View reviewed changes

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

most of my comments are fairly minor, overall I think this code is well written.

Sorry, something went wrong.

Uh oh!

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

gpshead Sep 30, 2022

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.

Describe what type this must be (usually bytes?) so that nobody is confused thinking it is a Python dict.

Consider using O! in your format below to have the API do a type check for you rather than failing later on when its use is attempted via the buffer API.

Sorry, something went wrong.

Uh oh!

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

All reactions

rhpvorderman Oct 2, 2022

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.

The problem with O! is that it cannot correctly accept bytes and bytearray simultaneously. Or is there a better solution for this?

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-bot added awaiting changes and removed awaiting review labels

Sep 30, 2022

bedevere-bot commented Sep 30, 2022

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

rhpvorderman added 5 commits

October 2, 2022 10:01


          Change char to bool

475aef6


          Properly bracketify if-else clause

41ba076


          Prefix underscore to _ZlibDecompressor name

5f1901d


          Copy explanation about zdict from python docs into function docstring

c5d6888


          Merge branch 'pythongh-95534' of github.com:rhpvorderman/cpython into p…

9d60339

…ythongh-95534

rhpvorderman commented Oct 2, 2022 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor Author

most of my comments are fairly minor, overall I think this code is well written.

As stated, most of the code is copied from the Cpython codebase is elsewhere, so I do not want to take credits for it. Thank you for the review. I have updated the code according to your comments.

EDIT: I did notice a significant oversight when revisiting: I have not added any testing code for _ZlibDecompressor. This can be very easily be arranged (Python-isal already has a full test suite). On the other hand _ZlibDecompressor's only use case is usage in the gzip module. So it can also be considered sufficiently tested using the current test suite.

EDIT: Added the test suite (adapted from test_bz2.py).

All reactions

Sorry, something went wrong.

Uh oh!

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

rhpvorderman force-pushed the gh-95534 branch from 332cf0a to 63a2623 Compare

October 3, 2022 08:30


          Add tests for _ZlibDecompressor

e3da415

rhpvorderman force-pushed the gh-95534 branch from 63a2623 to e3da415 Compare

October 3, 2022 08:31

gpshead added the sprint label

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

bedevere-bot added awaiting change review and removed awaiting changes labels

Oct 14, 2022

bedevere-bot commented Oct 14, 2022

Copy link

Copy Markdown

Thanks for making the requested changes!

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

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot requested a review from gpshead

October 14, 2022 11:17

gpshead approved these changes

Oct 17, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels

Oct 17, 2022

gpshead merged commit eae7dad into python:main

Oct 17, 2022

bedevere-bot removed the awaiting merge label

Oct 17, 2022

rhpvorderman deleted the gh-95534 branch

October 17, 2022 05:20

rhpvorderman mentioned this pull request

Oct 17, 2022

Give python-isal a mention in the zlib/gzip documentation #98347

Closed

carljm added a commit to carljm/cpython that referenced this pull request

Oct 17, 2022


          Merge branch 'main' into typewatch

cdbe93b

* main: (31 commits)
  pythongh-95913: Move subinterpreter exper removal to 3.11 WhatsNew (pythonGH-98345)
  pythongh-95914: Add What's New item describing PEP 670 changes (python#98315)
  Remove unused arrange_output_buffer function from zlibmodule.c. (pythonGH-98358)
  pythongh-98174: Handle EPROTOTYPE under macOS in test_sendfile_fallback_close_peer_in_the_middle_of_receiving (python#98316)
  pythonGH-98327: Reduce scope of catch_warnings() in _make_subprocess_transport (python#98333)
  pythongh-93691: Compiler's code-gen passes location around instead of holding it on the global compiler state (pythonGH-98001)
  pythongh-97669: Create Tools/build/ directory (python#97963)
  pythongh-95534: Improve gzip reading speed by 10% (python#97664)
  pythongh-95913: Forward-port int/str security change to 3.11 What's New in main (python#98344)
  pythonGH-91415: Mention alphabetical sort ordering in the Sorting HOWTO (pythonGH-98336)
  pythongh-97930: Merge with importlib_resources 5.9 (pythonGH-97929)
  pythongh-85525: Remove extra row in doc (python#98337)
  pythongh-85299: Add note warning about entry point guard for asyncio example (python#93457)
  pythongh-97527: IDLE - fix buggy macosx patch (python#98313)
  pythongh-98307: Add docstring and documentation for SysLogHandler.createSocket (pythonGH-98319)
  pythongh-94808: Cover `PyFunction_GetCode`, `PyFunction_GetGlobals`, `PyFunction_GetModule` (python#98158)
  pythonGH-94597: Deprecate child watcher getters and setters (python#98215)
  pythongh-98254: Include stdlib module names in error messages for NameErrors (python#98255)
  Improve speed. Reduce auxiliary memory to 16.6% of the main array. (pythonGH-98294)
  [doc] Update logging cookbook with an example of custom handling of levels. (pythonGH-98290)
  ...

rhpvorderman mentioned this pull request

Jan 25, 2023

gh-101322: Ensure test_zlib.ZlibDecompressorTest runs, fix errors in ZlibDecompressor #101323

Merged

qkaiser added a commit to onekey-sec/unblob that referenced this pull request

Oct 13, 2023


          fix(handlers): improve gzip reader according to recent changes in cpy…

7ea118f

…thon.

The gzip._GzipReader we're inheriting from had some changes to stay up
to date with changes in zlib library and to perform some optimization.

Specifically:

- GzipFile.read has been optimized. There is no longer a unconsumed_tail
  member to write back to padded file. This is instead handled by the
  ZlibDecompressor itself, which has an internal buffer.
- _add_read_data has been inlined, as it was just two calls.

We've adapted our own code to reflect these changes.

More info: python/cpython#97664

qkaiser added a commit to onekey-sec/unblob that referenced this pull request

Oct 13, 2023


          fix(handlers): improve gzip reader according to recent changes in cpy…

c09b4a0

…thon.

The gzip._GzipReader we're inheriting from had some changes to stay up
to date with changes in zlib library and to perform some optimization.

Specifically:

- GzipFile.read has been optimized. There is no longer a unconsumed_tail
  member to write back to padded file. This is instead handled by the
  ZlibDecompressor itself, which has an internal buffer.
- _add_read_data has been inlined, as it was just two calls.

We've adapted our own code to reflect these changes.

More info: python/cpython#97664

maurycy mentioned this pull request

Aug 8, 2025

gzip._GzipReader.read() with sometimes uninitialized variable #137571

Closed

picnixz mentioned this pull request

Aug 18, 2025

Faster decompression of gzip files #95534

Closed

Reviewers

Fidget-Spinner

Fidget-Spinner left review comments

gpshead

gpshead approved these changes

Uh oh!

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

Assignees

gpshead

Labels

extension-modules

C modules in the Modules dir

performance

Performance or resource usage

sprint

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

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.

GeezerVison

Conversation

rhpvorderman commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhpvorderman commented Sep 30, 2022

Uh oh!

Fidget-Spinner commented Sep 30, 2022

Uh oh!

rhpvorderman commented Sep 30, 2022

Uh oh!

Uh oh!

gpshead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gpshead Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

rhpvorderman Oct 2, 2022

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Sep 30, 2022

Uh oh!

rhpvorderman commented Oct 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhpvorderman commented Oct 14, 2022

Uh oh!

Fidget-Spinner commented Oct 14, 2022

Uh oh!

rhpvorderman commented Oct 14, 2022

Uh oh!

bedevere-bot commented Oct 14, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rhpvorderman commented Sep 30, 2022 •

edited

Loading

Uh oh!

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

rhpvorderman commented Oct 2, 2022 •

edited

Loading

Uh oh!

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