GeezerVison

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

GeezerVison is showing github.com — gh-113884: Refactor `queue.SimpleQueue` to use a ring buffer to store items by mpage · Pull Request #114259 · python/cpython

Conversation

mpage commented Jan 18, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Using a ring buffer instead of a Python list will simplify the process of making queue.SimpleQueue thread-safe in free-threaded builds. The ring buffer implementation has no places where critical sections may be released.

Issue: Make queue.SimpleQueue thread-safe in --disable-gil builds #113884

Sorry, something went wrong.

Uh oh!

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

All reactions


          Refactor queue.SimpleQueue to use a ring buffer to store items

b6c1407

Using a ring buffer instead of a Python list will simplify the
process of making `queue.SimpleQueue` thread-safe in free-threaded
builds. The ring buffer implementation has no places where critical
sections may be released.

bedevere-app Bot mentioned this pull request

Jan 18, 2024

Make queue.SimpleQueue thread-safe in --disable-gil builds #113884

Closed


          📜🤖 Added by blurb_it.

1ec875b

mpage marked this pull request as ready for review

January 18, 2024 20:59

mpage requested a review from rhettinger as a code owner

January 18, 2024 20:59

bedevere-app Bot added the awaiting review label

Jan 18, 2024

mpage mentioned this pull request

Jan 18, 2024

gh-113884: Make queue.SimpleQueue thread-safe in --disable-gil builds #114161

Merged

colesbury requested a review from erlend-aasland

January 18, 2024 21:07

colesbury approved these changes

Jan 18, 2024

View reviewed changes

colesbury left a comment

Copy link

Copy Markdown

Contributor

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.

LGTM.

@erlend-aasland, would you be able to review this?

Sorry, something went wrong.

Uh oh!

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

erlend-aasland reacted with thumbs up emoji

All reactions

👍 1 reaction

bedevere-app Bot added awaiting core review and removed awaiting review labels

Jan 18, 2024

erlend-aasland self-assigned this

Jan 18, 2024

erlend-aasland reviewed

Jan 18, 2024

View reviewed changes

erlend-aasland left a comment

Copy link

Copy Markdown

Contributor

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.

Thanks for doing the refactoring first!

Sorry, something went wrong.

Uh oh!

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

mpage reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

Comment thread

Modules/_queuemodule.c Resolved

Uh oh!

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

mpage added 5 commits

January 18, 2024 16:49


          Use a plain decref when clearing items

350306d


          Update comment for retrieving items from the ringbuf

63fa483


          Explicitly discard return value when shrinking ringbuf

ead8814


          Assert valid occupancy when adding to ringbuf

7e54296


          Add RingBuf APIs for empty/length

49956aa

To help reduce future churn.

rhettinger removed their request for review

January 19, 2024 04:12

mpage commented Jan 19, 2024

Copy link

Copy Markdown

Contributor Author

@erlend-aasland - I think I've addressed all of your comments. Would you please take another look?

erlend-aasland reacted with heart emoji

All reactions

❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

erlend-aasland added 2 commits

January 19, 2024 12:50


          NEWS is not needed; this is an internal change only

c472447


          Inline RingBuf_IsEmpty logic

9c0a58c

erlend-aasland added the skip news label

Jan 19, 2024

erlend-aasland enabled auto-merge (squash)

January 19, 2024 11:56

erlend-aasland commented Jan 19, 2024

Copy link

Copy Markdown

Contributor

Thanks a lot, @mpage and @colesbury 🙏

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland merged commit 28eacf2 into python:main

Jan 19, 2024

bedevere-app Bot removed the awaiting core review label

Jan 19, 2024

mpage deleted the gh-113884-simplequeue-ringbuf-refactor branch

January 19, 2024 17:38

kulikjak pushed a commit to kulikjak/cpython that referenced this pull request

Jan 22, 2024


          pythongh-113884: Refactor queue.SimpleQueue to use a ring buffer to…

208f0a7

… store items (python#114259)

Use a ring buffer instead of a Python list in order to simplify the
process of making queue.SimpleQueue thread-safe in free-threaded
builds. The ring buffer implementation has no places where critical
sections may be released.

aisk pushed a commit to aisk/cpython that referenced this pull request

Feb 11, 2024


          pythongh-113884: Refactor queue.SimpleQueue to use a ring buffer to…

fd8141d

… store items (python#114259)

Use a ring buffer instead of a Python list in order to simplify the
process of making queue.SimpleQueue thread-safe in free-threaded
builds. The ring buffer implementation has no places where critical
sections may be released.

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024


          pythongh-113884: Refactor queue.SimpleQueue to use a ring buffer to…

25a152f

… store items (python#114259)

Use a ring buffer instead of a Python list in order to simplify the
process of making queue.SimpleQueue thread-safe in free-threaded
builds. The ring buffer implementation has no places where critical
sections may be released.

Reviewers

erlend-aasland

erlend-aasland left review comments

colesbury

colesbury approved these changes

Uh oh!

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

Assignees

erlend-aasland

Labels

skip news

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.

3 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 01402520 — 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

mpage commented Jan 18, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mpage commented Jan 19, 2024

Uh oh!

erlend-aasland commented Jan 19, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mpage commented Jan 18, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

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