gh-102578: Optimise setting and deleting mutable attributes on non-dataclass subclasses of frozen dataclasses

[XuehaiPan]

Creating dataclasses with argument frozen=True will automatically generate methods __setattr__ and __delattr__ in _frozen_get_del_attr.

This PR changes tuple-based lookup to set-based lookup. Reduce the time complexity from $O(n)$ to $O(1)$.

In [1]: # tuple-basedIn [2]: %timeit'a'in ('a', 'b', 'c', 'd', 'e', 'f', 'g')
9.91ns ± 0.0982nsperloop (mean ± std. dev. of7runs, 100,000,000loopseach)
In [3]: %timeit'd'in ('a', 'b', 'c', 'd', 'e', 'f', 'g')
33.2ns ± 0.701nsperloop (mean ± std. dev. of7runs, 10,000,000loopseach)
In [4]: %timeit'g'in ('a', 'b', 'c', 'd', 'e', 'f', 'g')
56.4ns ± 0.818nsperloop (mean ± std. dev. of7runs, 10,000,000loopseach)
In [5]: # set-basedIn [6]: %timeit'a'in {'a', 'b', 'c', 'd', 'e', 'f', 'g'}
11.3ns ± 0.0723nsperloop (mean ± std. dev. of7runs, 100,000,000loopseach)
In [7]: %timeit'd'in {'a', 'b', 'c', 'd', 'e', 'f', 'g'}
11ns ± 0.106nsperloop (mean ± std. dev. of7runs, 100,000,000loopseach)
In [8]: %timeit'g'in {'a', 'b', 'c', 'd', 'e', 'f', 'g'}
11.1ns ± 0.126nsperloop (mean ± std. dev. of7runs, 100,000,000loopseach)

A tiny benchmark script:

fromcontextlibimportsuppressfromdataclassesimportFrozenInstanceError, dataclass@dataclass(frozen=True)classFoo2:
a: intb: intfoo2=Foo2(1, 2)
defbench2(inst):
withsuppress(FrozenInstanceError):
inst.a=0withsuppress(FrozenInstanceError):
inst.b=0@dataclass(frozen=True)classFoo7:
a: intb: intc: intd: inte: intf: intg: intfoo7=Foo7(1, 2, 3, 4, 5, 6, 7)
defbench7(inst):
withsuppress(FrozenInstanceError):
inst.a=0withsuppress(FrozenInstanceError):
inst.b=0withsuppress(FrozenInstanceError):
inst.c=0withsuppress(FrozenInstanceError):
inst.d=0withsuppress(FrozenInstanceError):
inst.e=0withsuppress(FrozenInstanceError):
inst.f=0withsuppress(FrozenInstanceError):
inst.g=0classBar(Foo7):
def__init__(self, a, b, c, d, e, f, g):
super().__init__(a, b, c, d, e, f, g)
self.baz=0defbench(inst):
inst.baz=1

Result:

set-based lookup:

In [2]: %timeitbench2(foo2)
1.08µs ± 28.1nsperloop (mean ± std. dev. of7runs, 1,000,000loopseach)
In [3]: %timeitbench7(foo7)
3.81µs ± 20.3nsperloop (mean ± std. dev. of7runs, 100,000loopseach)
In [4]: %timeitbench(bar)
249ns ± 6.31nsperloop (mean ± std. dev. of7runs, 1,000,000loopseach)

tuple-based lookup (original):

In [2]: %timeitbench2(foo2)
1.15µs ± 10.9nsperloop (mean ± std. dev. of7runs, 1,000,000loopseach)
In [3]: %timeitbench7(foo7)
3.97µs ± 15.7nsperloop (mean ± std. dev. of7runs, 100,000loopseach)
In [4]: %timeitbench(bar)
269ns ± 4.09nsperloop (mean ± std. dev. of7runs, 1,000,000loopseach)

The set-based is constantly faster than the old approach. And the theoretical time complexity is also smaller ($O(1)$ vs. $O(n)$).

• Issue: [Enhancement] Speed up setting and deleting mutable attributes on non-dataclass subclasses of frozen dataclasses #102578

Resolves#102578

[bedevere-bot]

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

[ghost]

All commit authors signed the Contributor License Agreement.

[AlexWaygood]

The issue-number CI check is failing because gh-102573 does not yet exist as a GitHub issue :)

You should create an issue to describe the change you're proposing here, and then link to it in the title of this PR.

[XuehaiPan]

Opened a linked issue and added some benchmark results.

[AlexWaygood]

Looks like the CLA check has started failing with the latest commit -- if you're using two email address, you may have to sign it with both email addresses, unfortunately :(

[XuehaiPan]

Seems that the CLA check has passed now.

[AlexWaygood]

Looks great to me. I verified the speedup by running this benchmark locally (which is a little different to the one you posted in your issue and the one you posted in this PR).

Benchmark:

importdataclassesimportstringimporttimeFoo=dataclasses.make_dataclass(
"Foo",
[(letter, int) forletterinstring.ascii_lowercase],
frozen=True
)
classBar(Foo): ...
instance=Bar(*range(26))
t0=time.perf_counter()
for_inrange(10_000_000):
instance.foo=1delinstance.fooprint(f"{time.perf_counter() -t0:.2f}")

The result of this benchmark is 15.15 seconds on main on my machine (--pgo non-debug build), but 6.44 seconds with this patch applied.

I'll wait for a thumbs-up from @ericvsmith, @carljm, or another core dev before merging, but this has my approval -- thanks!

[ericvsmith]

Not that it matters much, but the zero length case could just avoid the or name in ... test entirely. Maybe fields_str should become fields_test, and then set it to or name in {<<generated set literal>>} or set it to an empty string if there are no fields. Then change the generated code to f'if type(self) is cls {fields_test}:' Although that doesn't read very well. Maybe tweak fields_test to be something else.

This could be part of a different PR, or include it here. But in any event I'm not positive that the zero length case actually has a test. We should make sure it does for this PR.

[XuehaiPan]

But in any event I'm not positive that the zero length case actually has a test. We should make sure it does for this PR.

Added a small test for empty frozen dataclass.

[carljm]

Code changes LGTM. A couple comments on the new test.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

[XuehaiPan]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@carljm, @AlexWaygood: please review the changes made to this pull request.

[carljm]

Looks good to me; thanks for the perf improvement!

[AlexWaygood]

Looks great!