Skip to content

gh-134938: Add set_pledged_input_size to ZstdCompressor #135010

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

Merged
merged 17 commits into from
Jun 5, 2025

Conversation

emmatyping
Copy link
Member

@emmatyping emmatyping commented Jun 1, 2025

This PR adds a new method set_pledged_input_size(), which allows users to declare up front how much data will be written to the compressor for a given frame. This combined with CompressionParameter.content_size_flag allows users to ensure that even for streaming compression scenarios the content size is written into the zstd frame header, which is beneficial for reducing decompression memory usage.


📚 Documentation preview 📚: https://cpython-previews--135010.org.readthedocs.build/

@emmatyping
Copy link
Member Author

cc @Rogdham

@@ -247,6 +247,25 @@ Compressing and decompressing data in memory
The *mode* argument is a :class:`ZstdCompressor` attribute, either
:attr:`~.FLUSH_BLOCK`, or :attr:`~.FLUSH_FRAME`.

.. method:: set_pledged_input_size(size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't currently explain why one would want to use the method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a bit to describe this, but suggestions welcome.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that mark up should be cleaned up, but there are many occurrences of ~. in existing documentation, so this can be done in a following PR.

Otherwise LGTM. 👍

@emmatyping
Copy link
Member Author

@serhiy-storchaka
Thanks for the approval!

I think that mark up should be cleaned up, but there are many occurrences of ~. in existing documentation, so this can be done in a following PR.

I'll make a follow up after this is merged to resolve these.

@serhiy-storchaka serhiy-storchaka merged commit 4b44b34 into python:main Jun 5, 2025
39 checks passed
@miss-islington-app
Copy link

Thanks @emmatyping for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 5, 2025
…onGH-135010)

(cherry picked from commit 4b44b34)

Co-authored-by: Emma Smith <emma@emmatyping.dev>
@bedevere-app
Copy link

bedevere-app bot commented Jun 5, 2025

GH-135173 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 5, 2025
serhiy-storchaka pushed a commit that referenced this pull request Jun 5, 2025
…135010) (GH-135173)

(cherry picked from commit 4b44b34)

Co-authored-by: Emma Smith <emma@emmatyping.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants