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

gh-140232: Do not track frozenset objects with immutables#140234

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
vstinner merged 46 commits intopython:mainfromeendebakpt:frozenset_immutable_tracking
Jan 28, 2026

Conversation

@eendebakpt
Copy link
Contributor

@eendebakpteendebakpt commentedOct 16, 2025•
edited
Loading

In the PR we untrack frozen tuples for the normal constructors. There are a few methods shared between theset andfrozenset (for exampleset_intersection insetobject.c) where we have not added the untracking. (this is possible, but I am not sure this is worthwhile to do).

Here is a small script to test the idea:

importgcimporttimefromstatisticsimportmeannumber_of_iterations=20number_of_gc_iterations=50deltas= []gc.disable()gc.collect()forkkinrange(number_of_iterations):t0=time.perf_counter()forjjinrange(number_of_gc_iterations):gc.collect()dt=time.perf_counter()-t0deltas.append(dt)print(f"time per collection: mean{1e3*mean(deltas)/number_of_iterations:.3f} [ms], min{1e3*min(deltas)/number_of_iterations:.3f} [ms]")sets= [frozenset([ii])foriiinrange(10_000)]deltas= []print("---")gc.disable()gc.collect()forkkinrange(number_of_iterations):t0=time.perf_counter()forjjinrange(number_of_gc_iterations):gc.collect()dt=time.perf_counter()-t0deltas.append(dt)print(f"time per collection: mean{1e3*mean(deltas)/number_of_iterations:.3f} [ms], min{1e3*min(deltas)/number_of_iterations:.3f} [ms]")#%% Show statistics of frozen containersgc.collect()defcandidate(obj):returnall(notgc.is_tracked(x)forxinobj)forimmutable_typein (tuple,frozenset):number_of_objects_tracked=0number_of_candidates=0number_of_immutable_candidates=0forobjingc.get_objects():number_of_objects_tracked+=1iftype(obj)isimmutable_type:number_of_candidates+=1# print(f"{type(obj)} = {obj}")ifcandidate(obj):number_of_immutable_candidates+=1print(f"type{immutable_type}")print(f"{number_of_objects_tracked=}")print(f"{number_of_candidates=}")print(f"{number_of_immutable_candidates=}")

It measures the performance of garbage collection, and outputs some statistics for the numbers of frozen containers.

Main:

time per collection: mean 1.311 [ms], min 1.301 [ms]---time per collection: mean 2.467 [ms], min 2.272 [ms]type <class 'tuple'>  number_of_objects_tracked=18330  number_of_candidates=546  number_of_immutable_candidates=1type <class 'frozenset'>  number_of_objects_tracked=18330  number_of_candidates=10059  number_of_immutable_candidates=10057

PR

time per collection: mean 1.285 [ms], min 1.251 [ms]---time per collection: mean 1.424 [ms], min 1.396 [ms]type <class 'tuple'>  number_of_objects_tracked=8273  number_of_candidates=546  number_of_immutable_candidates=6type <class 'frozenset'>  number_of_objects_tracked=8273  number_of_candidates=2  number_of_immutable_candidates=0

Note: generative ai was used in creating the PR

Co-authored-by: Mikhail Efimov <efimov.mikhail@gmail.com>
@sergey-miryanov
Copy link
Contributor

sergey-miryanov commentedOct 17, 2025•
edited by efimov-mikhail
Loading

Maybe it is worth to changetp_alloc for something like:

PyObject*PyFrozenSet_Alloc(PyTypeObject*type,Py_ssize_tnitems){PyObject*obj=PyType_GenericAlloc(type,nitems);if (obj==NULL) {returnNULL;    }_PyFrozenSet_MaybeUntrack(obj);returnobj;}

@eendebakpt
Copy link
ContributorAuthor

Maybe it is worth to changetp_alloc for something like:

Thetp_alloc is used inmake_new_set, which in turn is called bymake_new_set. The last one is usedset_intersection whichmodifies afrozenset. So adding _PyFrozenSet_MaybeUntrack totp_alloc would mean we have to add a_PyFrozenSet_MaybeTrack to the end ofset_intersection. This is a complication I do not want to tackle (certainly not in this PR).

sergey-miryanov reacted with thumbs up emoji

Copy link
Contributor

@sergey-miryanovsergey-miryanov left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

@vstinner
Copy link
Member

Would it be possible to write tests in Python rather than in C?

@eendebakpt
Copy link
ContributorAuthor

Would it be possible to write tests in Python rather than in C?

I tried, but it is not easy. We have to exposePySet_Add (frozenset().add does not exist on the python side). I addedpyset_add on the_testcapi module (withpyset_add just callingPySet_Add). But running this on a frozenset from the python side does not work: when calling_testcapi.pyset_add(frozen_set, item) there too many references to thefrozen_set andPySet_Add will fail with an internal error here:

PyErr_BadInternalCall();

And when calling_testcapi.pyset_add(frozenset(), item) we do not have the frozenset available to test whether tracking has been enabled.

@sergey-miryanov
Copy link
Contributor

And when calling_testcapi.pyset_add(frozenset(), item) we do not have the frozenset available to test whether tracking has been enabled.

IIUC, if you return the first argument from pyset_add then you can test it on the python side.

@eendebakpt
Copy link
ContributorAuthor

And when calling_testcapi.pyset_add(frozenset(), item) we do not have the frozenset available to test whether tracking has been enabled.

IIUC, if you return the first argument from pyset_add then you can test it on the python side.

Ok, I gave it another try. The first attempt failed, but by using the vectorcall convention I can keep the reference count at 1 also from the Python side.

Co-authored-by: Victor Stinner <vstinner@python.org>
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM. With one last comment :-)

@eendebakpt
Copy link
ContributorAuthor

LGTM. With one last comment :-)

Nice. Give me one second to double check something.

Copy link
Member

@efimov-mikhailefimov-mikhail left a comment

Choose a reason for hiding this comment

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

LGTM

@eendebakpt
Copy link
ContributorAuthor

LGTM. With one last comment :-)

Nice. Give me one second to double check something.

The_PyFrozenSet_MaybeUntrack is only used in two places and in one of then the argument is guaranteed to be an exact frozenset. I checked whether we can simplify code by changing theif (!PyFrozenSet_CheckExact(op)) ... into an assert. It can be done, but then we have to add other check inmake_new_frozenset so overall it is not clear improvement.

PR is ready from my side!

Copy link
Contributor

@sergey-miryanovsergey-miryanov left a comment

Choose a reason for hiding this comment

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

Two nitpicks. Otherwise LGTM.

@vstinnervstinner merged commit08d7bd2 intopython:mainJan 28, 2026
47 checks passed
@vstinner
Copy link
Member

Merged, thanks for the many updates!

efimov-mikhail reacted with hooray emoji

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

Reviewers

@sergey-miryanovsergey-miryanovsergey-miryanov approved these changes

@vstinnervstinnervstinner approved these changes

@efimov-mikhailefimov-mikhailefimov-mikhail approved these changes

@rhettingerrhettingerAwaiting requested review from rhettingerrhettinger is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrently

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eendebakpt@sergey-miryanov@vstinner@efimov-mikhail
[8]ページ先頭
©2009-2026 Movatter.jp