Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Draft: gh-123471: Make concurrent iteration over itertools.pairwise safe under free-threading#125417

Closed
eendebakpt wants to merge 5 commits intopython:mainfrom
eendebakpt:pairwise_v2
Closed

Draft: gh-123471: Make concurrent iteration over itertools.pairwise safe under free-threading#125417
eendebakpt wants to merge 5 commits intopython:mainfrom
eendebakpt:pairwise_v2

Conversation

@eendebakpt
Copy link
Contributor

@eendebakpteendebakpt commentedOct 13, 2024
edited
Loading

We make concurrent iteration overitertools.pairwise safe by:

  • Using_PyObject_IsUniquelyReferenced for re-use of the result type
  • Holding a reference topo->it instead of a borrowed reference
  • Obtaining a reference topo->old early.

Also see#123848

To makepairwise_next safe we need to prevent concurrent iterators from accessing the clearedpo->it

  • Idea 1: In the first line ofpairwise_next replacePyObject *it = po->it withPyObject *it = Py_XInRef(po->it);.
    Then every thread will have a true reference to the iterator, and not a borrowed reference.
    Simple, but overhead is an incref/decref in the common case
  • Idea 2a: Do not clearpo->it. We then need another way to signal that the iterator has been exhausted.
    Use another variable "int iterator_exhausted" in the pairwise object. Requires a bit more memory
    for each pairwise object, but the check is a simple int comparison.
  • Idea 2b: Do not clearpo->it. We then need another way to signal that the iterator has been exhausted.
    Usepo->old to set a sentinel. Requires a global sentinel in the module (or a sentinel per thread).
    In the top ofpairwise_next we would have
PyObject *old = Py_XNewRef(po->old);if (old == sentinel) {      // iterator was exhausted earlier, return NULL      Py_DECREF(old)      return NULL;}if (old == NULL) {      // first call to pairwise_next, normal code      ...}

An issue with this approach is that at the end of pairwise_next there is the line:Py_XSETREF(po->old, new);
So one thread can set the sentinel, but in another thread the value could be overwritten.
Maybe this is ok: this will not cause crashes, and we do not have to guarantee correct behaviour for
the free-threading build.
This approach does not require additional increfs/decrefs, but it does require setting up the sentinel.

In this PR we work out the first option.

@rhettinger
Copy link
Contributor

Can I suggest that you start withenumerate() rather thanpairwise(). The former is a built-in, so more people care about it and your edit will get more attention, feedback, and buy-in.

Also, I would like to keep the latter as simple as possible for now. I'm evaluating a proposal to addprefix andsuffix options topairwise(). This would bemuch harder to do after these edits.

@eendebakpt
Copy link
ContributorAuthor

Can I suggest that you start withenumerate() rather thanpairwise(). The former is a built-in, so more people care about it and

Sure! I created a PR forenumerate at#125734. Forreversed there was an earlier PR#120971 where your review was requested. Would you have time to review that one?

@eendebakpt
Copy link
ContributorAuthor

Closing in favor of#144489

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@rhettingerrhettingerAwaiting requested review from rhettingerrhettinger is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eendebakpt@rhettinger

[8]ページ先頭

©2009-2026 Movatter.jp