◐ Shell
reader mode source ↗
Skip to content

gh-132983: Convert dict_content to take Py_buffer#133924

Merged
AA-Turner merged 6 commits into
python:mainfrom
AA-Turner:zstd-dict-content-buffer
May 26, 2025
Merged

gh-132983: Convert dict_content to take Py_buffer#133924
AA-Turner merged 6 commits into
python:mainfrom
AA-Turner:zstd-dict-content-buffer

Conversation

@AA-Turner

@AA-Turner AA-Turner commented May 12, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

ZstdDict's dict_content is just binary data, we don't need to allocate an entire PyBytesObject for it. This also lets us use the Py_buffer argument clinic converter.

We replace the dict_content member of ZstdDict with dict_buffer and dict_len. ZstdDict.dict_content is still exposed as a bytes object to Python via a new getter.

A

cc @Rogdham

@AA-Turner AA-Turner requested a review from emmatyping May 12, 2025 12:26
@bedevere-app bedevere-app Bot mentioned this pull request May 12, 2025
Closed
13 tasks
@serhiy-storchaka serhiy-storchaka self-requested a review May 12, 2025 15:01
@emmatyping

emmatyping commented May 12, 2025

Copy link
Copy Markdown
Member

It would probably be good to run the refleaks buildbots on this PR before merge.

@emmatyping

emmatyping commented May 23, 2025

Copy link
Copy Markdown
Member

@AA-Turner would you like to pick this up again? I'm happy to take this over if you don't have time. Want to get the last of the zstd items done before b2.

@AA-Turner AA-Turner added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 24, 2025
@bedevere-bot

bedevere-bot commented May 24, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @AA-Turner for commit fe3fb49 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133924%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 24, 2025
@emmatyping

emmatyping commented May 24, 2025

Copy link
Copy Markdown
Member

Merged main because there were nonsense errors in the tests.

@AA-Turner AA-Turner force-pushed the zstd-dict-content-buffer branch from 90cef43 to 30ac9d5 Compare May 26, 2025 07:55
@AA-Turner AA-Turner added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 26, 2025
@bedevere-bot

bedevere-bot commented May 26, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @AA-Turner for commit 30ac9d5 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133924%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 26, 2025
@AA-Turner

AA-Turner commented May 26, 2025

Copy link
Copy Markdown
Member Author

Apologies for the force-push, my local git state got messy.

serhiy-storchaka
serhiy-storchaka approved these changes May 26, 2025
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

There is no principal difference between keeping binary data as a bytes object or a raw memory buffer. But if the latter looks better to you, LGTM.

@AA-Turner AA-Turner enabled auto-merge (squash) May 26, 2025 14:30
Hide details View details @AA-Turner AA-Turner merged commit f2ce4bb into python:main May 26, 2025
38 checks passed
@miss-islington-app

miss-islington-app Bot commented May 26, 2025

Copy link
Copy Markdown

Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2025
@AA-Turner @miss-islington
pythongh-132983: Convert dict_content to take Py_buffer in ``ZstdDict… …
e480f51 
…()`` (pythonGH-133924)

(cherry picked from commit f2ce4bb)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented May 26, 2025

Copy link
Copy Markdown

GH-134723 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 May 26, 2025
AA-Turner added a commit that referenced this pull request May 28, 2025
@miss-islington @AA-Turner
[3.14] gh-132983: Convert dict_content to take Py_buffer in ``ZstdDic… …
4841201 
…t()`` (GH-133924) (#134723)

gh-132983: Convert dict_content to take Py_buffer in ``ZstdDict()`` (GH-133924)
(cherry picked from commit f2ce4bb)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@AA-Turner @Pranjal095
pythongh-132983: Convert dict_content to take Py_buffer in ``ZstdDict… …
b88a24e 
…()`` (python#133924)
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@AA-Turner @taegyunkim
pythongh-132983: Convert dict_content to take Py_buffer in ``ZstdDict… …
ed0a28b 
…()`` (python#133924)
gostak-dd pushed a commit to gostak-dd/cpython that referenced this pull request Jun 2, 2026
@AA-Turner @gostak-dd
pythongh-132983: Convert dict_content to take Py_buffer in ``ZstdDict… …
c94c79e 
…()`` (python#133924)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner

@emmatyping emmatyping emmatyping left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AA-Turner @emmatyping @bedevere-bot @vstinner @serhiy-storchaka