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-89550: Buffer GzipFile.write to reduce execution time by ~15% by CCLDArjun · Pull Request #101251 · python/cpython

Conversation

@CCLDArjun

@CCLDArjun CCLDArjun commented Jan 22, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Currently, all of this is done for every write() call:
https://github.com/python/cpython/blob/main/Lib/gzip.py#L266-L288

This pr stores GzipFile.write() data into a buffer to postpone most of the compression work to when the buffer flushes. This way, we can call into zlib and File.write() less frequently.

benchmarking with timeit:

../python -m timeit -s 'import gzip
' 'with gzip.open("./Python-3.11.1.tgz", "rb") as infile:
' '    with gzip.open("./Python-3.11.1new.tgz", "wb") as outfile:
' '        for line in infile:
' '            outfile.write(line)
' '
'
1 loop, best of 5: 6.36 sec per loop

vs

../python -m timeit -s 'import gzipold as gzip
' 'with gzip.open("./Python-3.11.1.tgz", "rb") as infile:
' '    with gzip.open("./Python-3.11.1new.tgz", "wb") as outfile:
' '        for line in infile:
' '            outfile.write(line)
' '
'
1 loop, best of 5: 7.68 sec per loop
  • Issue: GzipFile.write should be buffered #89550

Sorry, something went wrong.

Uh oh!

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

rhpvorderman and marcelm reacted with hooray emoji rhpvorderman reacted with heart emoji
All reactions
  • 🎉 2 reactions
  • ❤️ 1 reaction
CCLDArjun added 4 commits January 22, 2023 09:26
@CCLDArjun
initial testing to see if it's faster
943ca9c
@CCLDArjun
Merge branch 'python:main' into buffered-gzip-writes
6f4701b
@CCLDArjun
use io.BufferedWriter to buffer gzip writes
8653faf
@CCLDArjun
:Merge branch 'buffered-gzip-writes' of https://github.com/ccldarjun/…
1c36625
…cpython into buffered-gzip-writes
@bedevere-bot bedevere-bot added the awaiting review label Jan 22, 2023
@bedevere-bot

bedevere-bot commented Jan 22, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

All reactions

Sorry, something went wrong.

Uh oh!

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

@CCLDArjun CCLDArjun changed the title Buffer gzip writes to reduce execution time by ~15% gh-89550: Buffer GzipFile.write to reduce execution time by ~15% Jan 22, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jan 22, 2023
GzipFile.write should be buffered #89550
Closed
@AlexWaygood AlexWaygood added the performance Performance or resource usage label Jan 22, 2023
@CCLDArjun
news blurb
bc4d929
@rhpvorderman

rhpvorderman commented Jan 24, 2023

Copy link
Copy Markdown
Contributor

Wow @CCLDArjun such a simple and effective solution. My solution was to split out a _GzipWriter (just like _GzipReader) and try to work from there, but I never got that to work easily and it was quite some extra code too. I am so glad you made this. I hope this will get merged soon!

CCLDArjun reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

@CCLDArjun

CCLDArjun commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

My solution was to split out a _GzipWriter (just like _GzipReader) and try to work from there

Initially that was my idea too, where I subclassed _pyio.BufferedWriter to override its flush methods but that also wasn't working easily and was pretty messy: 943ca9c

All reactions

Sorry, something went wrong.

Uh oh!

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

@rhpvorderman

rhpvorderman commented Jan 25, 2023

Copy link
Copy Markdown
Contributor

I am glad that you were able to come up with a better idea. I have been hacking at the gzip module pretty thoroughly but I could not come up with this. I am in awe 😀 . I will port this to python-isal and I have recently started on making a zlib-ng module too. That way people will not have to wait for 3.12 to ship to get all this fast compression.

All reactions

Sorry, something went wrong.

Uh oh!

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

@rhpvorderman rhpvorderman mentioned this pull request Jan 25, 2023
Buffer compression writers pycompression/xopen#78
Merged
@CCLDArjun
Merge branch 'main' into buffered-gzip-writes
ef9dd50
@orsenthil

orsenthil commented Jan 27, 2023

Copy link
Copy Markdown
Member

This looks good to me. A core developer in the expert list will review before merging.

All reactions

Sorry, something went wrong.

Uh oh!

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

This was referenced Feb 6, 2023
Buffer writing gzipfile pycompression/python-isal#131
Closed
Buffered writing for GzipFile pycompression/python-zlib-ng#8
Closed
@rhpvorderman

rhpvorderman commented Feb 8, 2023

Copy link
Copy Markdown
Contributor

@orsenthil This is still labeled "awaiting review" rather then "awaiting core review" does that matter?

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev

arhadthedev commented Feb 8, 2023

Copy link
Copy Markdown
Member

@rhpvorderman Only review messages from the Python team count.

@Yhg1s, @gpshead as zipfile experts (not quite the same but still).

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev arhadthedev added the stdlib Standard Library Python modules in the Lib/ directory label Feb 8, 2023
@svisser svisser mentioned this pull request Feb 17, 2023
Slow performance due to lack of buffering for GzipFile.write piskvorky/smart_open#760
Closed
@gpshead
gpshead self-requested a review February 17, 2023 23:54
gpshead
gpshead requested changes Feb 18, 2023
View reviewed changes

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

Overall I like this change. It's great how cleanly this can be added. :)

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/gzip.py Resolved

Uh oh!

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

Comment thread Lib/gzip.py Resolved

Uh oh!

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

Comment thread Lib/gzip.py Resolved

Uh oh!

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

@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Feb 18, 2023
@bedevere-bot

bedevere-bot commented Feb 18, 2023

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.

@gpshead gpshead self-assigned this Feb 18, 2023
CCLDArjun added 2 commits February 17, 2023 23:15
@CCLDArjun
added buffer_size for file writes
b3aa1f7
@CCLDArjun
:Merge branch 'buffered-gzip-writes' of https://github.com/ccldarjun/…
ad2dc9b
…cpython into buffered-gzip-writes
@CCLDArjun

CCLDArjun commented Feb 22, 2023

Copy link
Copy Markdown
Contributor Author

@gpshead tests are passing! Can you re review?

All reactions

Sorry, something went wrong.

Uh oh!

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

svisser
svisser reviewed Feb 23, 2023
View reviewed changes
Comment thread Lib/gzip.py Resolved

Uh oh!

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

CCLDArjun added 2 commits February 23, 2023 14:54
@CCLDArjun
use constant for write buffer size
fe31588
@CCLDArjun
Merge branch 'main' into buffered-gzip-writes
875ebd8
@rhpvorderman

rhpvorderman commented May 8, 2023

Copy link
Copy Markdown
Contributor

ping

It would be great if this gets into 3.12. The measured execution difference of 15% is at gzip level 9 which is the default. At a more modest compression level such as 1, the difference can be a couple of hundreds of percents.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood
Merge branch 'main' into buffered-gzip-writes
e33fc19
@CCLDArjun

CCLDArjun commented May 8, 2023

Copy link
Copy Markdown
Contributor Author

pinging @gpshead for a final review

All reactions

Sorry, something went wrong.

Uh oh!

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

@gpshead
improve news wording and ReSTify
5ef4674
gpshead
gpshead approved these changes May 8, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels May 8, 2023
@gpshead
Merge branch 'main' into buffered-gzip-writes
c0f3d2a
@gpshead
gpshead enabled auto-merge (squash) May 8, 2023 17:29
@gpshead
gpshead merged commit 9af4854 into python:main May 8, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 8, 2023
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@CCLDArjun @AlexWaygood
@gpshead @jbower-fb
pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (
a49c8a8
python#101251)

Use `io.BufferedWriter` to buffer gzip writes.

---------

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Gregory P. Smith <[email protected]>
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
@carljm
Merge branch 'main' into inlinecomp2
76077cd
* main: (47 commits)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)
  pythongh-103193: Improve `getattr_static` test coverage (python#104286)
  Trim trailing whitespace and test on CI (python#104275)
  pythongh-102500: Remove mention of bytes shorthand (python#104281)
  pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)
  pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)
  pythongh-104273: Remove redundant len() calls in argparse function (python#104274)
  pythongh-64660: Don't hardcode Argument Clinic return converter result variable name (python#104200)
  pythongh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (python#104266)
  pythonGH-102613: Improve performance of `pathlib.Path.rglob()` (pythonGH-104244)
  pythongh-103650: Fix perf maps address format (python#103651)
  pythonGH-89812: Churn `pathlib.Path` methods (pythonGH-104243)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
@carljm
Merge branch 'main' into fixsendfamily
a614216
* main: (29 commits)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)
  pythongh-103193: Improve `getattr_static` test coverage (python#104286)
  Trim trailing whitespace and test on CI (python#104275)
  pythongh-102500: Remove mention of bytes shorthand (python#104281)
  pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)
  pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
@carljm
Merge branch 'main' into supermon
c48eee3
* main: (156 commits)
  pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)
  pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)
  pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)
  pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)
  pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)
  pythongh-104240: return code unit metadata from codegen (python#104300)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
@carljm
Merge branch 'main' into fixloadsuperspec
cf11908
* main: (35 commits)
  pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)
  pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)
  pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)
  pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)
  pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)
  pythongh-104240: return code unit metadata from codegen (python#104300)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  ...
@bdarnell bdarnell mentioned this pull request Jun 15, 2023
GzipFile.flush doesn't flush compressor in 3.12 beta #105808
Closed
Yhg1s added a commit to Yhg1s/cpython that referenced this pull request Jun 19, 2023
@Yhg1s
pythonGH-105808: Fix a regression introduced in pythonGH-101251, causing
b7ecf06
GzipFile.flush() to not flush the compressor (nor pass along the zip_mode
argument).
Yhg1s added a commit to Yhg1s/cpython that referenced this pull request Jun 19, 2023
@Yhg1s
Fix a regression introduced in pythonGH-101251, causing GzipFile.flus…
5b3a658
…h() to

not flush the compressor (nor pass along the zip_mode argument).
@Yhg1s Yhg1s mentioned this pull request Jun 19, 2023
GH-105808: Fix a regression introduced in GH-101251 #105910
Merged
gpshead pushed a commit that referenced this pull request Jun 19, 2023
@Yhg1s
GH-105808: Fix a regression introduced in GH-101251 (#105910)
1858db7
Fix a regression introduced in GH-101251, causing GzipFile.flush() to
not flush the compressor (nor pass along the zip_mode argument).
@miss-islington miss-islington mentioned this pull request Jun 19, 2023
[3.12] GH-105808: Fix a regression introduced in GH-101251 (GH-105910) #105920
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 19, 2023
@Yhg1s @miss-islington
pythonGH-105808: Fix a regression introduced in pythonGH-101251 (pyth…
15b4976
…onGH-105910)

Fix a regression introduced in pythonGH-101251, causing GzipFile.flush() to
not flush the compressor (nor pass along the zip_mode argument).
(cherry picked from commit 1858db7)

Co-authored-by: T. Wouters <[email protected]>
Yhg1s added a commit that referenced this pull request Jun 19, 2023
@miss-islington @Yhg1s
[3.12] GH-105808: Fix a regression introduced in GH-101251 (GH-105910) (
225cc4c
#105920)

GH-105808: Fix a regression introduced in GH-101251 (GH-105910)

Fix a regression introduced in GH-101251, causing GzipFile.flush() to
not flush the compressor (nor pass along the zip_mode argument).
(cherry picked from commit 1858db7)

Co-authored-by: T. Wouters <[email protected]>
@effigies effigies mentioned this pull request Aug 18, 2023
GzipFile.seek makes invalid write if buffer is not flushed in Python 3.12rc1 #108111
Closed
2 tasks done
@qiyunzhu qiyunzhu mentioned this pull request Nov 17, 2023
Scikit-bio installation with Python 3.12 causes failures when testing scikit-bio/scikit-bio#1897
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
+1 more reviewer
@svisser svisser svisser left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@gpshead gpshead

Labels

performance Performance or resource usage stdlib Standard Library Python modules in the Lib/ directory

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.

8 participants

@CCLDArjun @bedevere-bot @rhpvorderman @orsenthil @arhadthedev @gpshead @svisser @AlexWaygood
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.