[ty] Make `NominalInstanceType` internally wrap an enum #19811

AlexWaygood · 2025-08-07T17:20:14Z

Stacked on top of #19669; review that PR first!

Summary

This implements @carljm's suggestion from #19669 (comment), which appears to buy back almost all the performance and memory regressions from #19669. If we like this approach, I'll merge this PR into that branch shortly before merging that PR. (I'm keeping them as separate PRs for now, for ease of review.)

Test Plan

All existing tests pass. I don't believe this has any impact on semantics. There is still a small primer report on this PR, but it's now a bit of a mystery to me: I can't reproduce any of the "diagnostics going away" on the target branch of this PR, and I can reproduce all of the "diagnostics being added" on the target branch of this PR. I've seen slightly confusing results from mypy_primer in the past for stacked PRs; I suspect there's a subtle bug in the workflow somewhere that leads to it reporting a diagnostic diff even when there isn't one in cases like this.

github-actions · 2025-08-07T17:23:39Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

github-actions · 2025-08-07T17:41:27Z

`mypy_primer` results

No ecosystem changes detected ✅

Memory usage changes were detected when running on open source projects

flake8 (https://github.com/pycqa/flake8)
-     struct metadata = ~3MB
+     struct metadata = ~2MB

trio (https://github.com/python-trio/trio)
-     struct metadata = ~8MB
+     struct metadata = ~7MB

sphinx (https://github.com/sphinx-doc/sphinx)
-     struct fields = ~16MB
+     struct fields = ~15MB

prefect (https://github.com/PrefectHQ/prefect)
-     struct fields = ~35MB
+     struct fields = ~33MB
-     memo metadata = ~84MB
+     memo metadata = ~80MB

codspeed-hq · 2025-08-07T17:43:26Z

CodSpeed WallTime Performance Report

Merging #19811 will improve performances by 63.76%

_{Comparing alex/remove-tuples-instance-enum (dd260ae) with alex/remove-tuples (3436d5d)}

Summary

⚡ 4 improvements
✅ 3 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`large[sympy]`	41.3 s	39.7 s	+4.08%
⚡	`medium[colour-science]`	10.6 s	6.5 s	+63.76%
⚡	`medium[pandas]`	26.4 s	24.7 s	+6.94%
⚡	`small[tanjun]`	1.7 s	1.6 s	+5.65%

codspeed-hq · 2025-08-07T17:43:45Z

CodSpeed Instrumentation Performance Report

Merging #19811 will improve performances by 6.26%

_{Comparing alex/remove-tuples-instance-enum (dd260ae) with alex/remove-tuples (3436d5d)}

Summary

⚡ 1 improvements
✅ 41 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`ty_micro[many_string_assignments]`	73.9 ms	69.6 ms	+6.26%

AlexWaygood · 2025-08-08T17:47:06Z

Well... there's some bad news here. I just noticed that mypy_primer runs seem to be taking a very long time on this PR branch. It looks like it takes 10 minutes to type-check static-frame with this PR branch in mypy_primer, whereas it takes around 1 minute to type-check static-frame with #19669. I can reproduce a severe slowdown locally on this branch for that repo too. No idea why that would be yet.

AlexWaygood · 2025-08-08T19:08:35Z

Okay, fbcb8e3 fixes the static-frame perf regression for me locally! It might be worthwhile for us to add static-frame as a benchmark -- I can look into that as a followup.

Edit: and it fixed it in CI as well -- mypy_primer is back to completing in ~3m!

This reverts commit 252a603.

AlexWaygood · 2025-08-10T10:03:33Z

crates/ty_python_semantic/src/types/tuple.rs

+    // N.B. If this method is not Salsa-tracked, we take 10 minutes to check
+    // `static-frame` as part of a mypy_primer run! This is because it's called
+    // from `NominalInstanceType::class()`, which is a very hot method.
+    #[salsa::tracked]


See #19669 (comment)

AlexWaygood · 2025-08-10T10:07:29Z

crates/ty_python_semantic/src/types/instance.rs

+    /// If this is an instance type where the class has a tuple spec, returns the tuple spec.
+    ///
+    /// I.e., for the type `tuple[int, str]`, this will return the tuple spec `[int, str]`.
+    /// For a subclass of `tuple[int, str]`, it will return the same tuple spec.
+    pub(super) fn tuple_spec(&self, db: &'db dyn Db) -> Option<Cow<'db, TupleSpec<'db>>> {


I moved this method from class.rs to instance.rs so that we could avoid having to materialize the ClassType if the instance type was an ExactTuple variant internally.

Now that I've added caching to TupleType::to_class_type(), this might not be a particularly significant optimisation anymore. But with our current callsites across the repo, we only ever need to know the tuple spec of instances, never the tuple spec of Type::GenericAliases. So there doesn't appear to be a huge amount of downside to making this a method on NominalInstanceType rather than a method on ClassType.

slice_literal could also possibly be made a method on NominalInstanceType rather than ClassType, but it doesn't seem to have any performance benefits from what I can tell locally, and it would make this PR's diff bigger.

AlexWaygood · 2025-08-10T15:28:05Z

crates/ty_python_semantic/src/types/tuple.rs

-                        .apply_specialization(db, |_| generic_context.specialize_tuple(db, self)),
-                ),
-            })
+            .expect("Typeshed should always have a `tuple` class in `builtins.pyi`");


This .expect() call means we now panic if we're given a custom typeshed without a tuple class. I don't love this but I also think it's okay -- there are already classes where we panic if they're not available in typeshed, such as object and types.ModuleType. tuple is similarly special; I think it's okay if we effectively mandate that custom typesheds have to have a tuple class in them.

Doing this refactor without the .expect() call here would be quite a bit harder, I think.

AlexWaygood · 2025-08-11T10:52:42Z

I've merged the changes here directly into #19669

AlexWaygood added the ty Multi-file analysis & type inference label Aug 7, 2025

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch from 055747d to f010554 Compare August 7, 2025 17:21

AlexWaygood mentioned this pull request Aug 7, 2025

[ty] Remove Type::Tuple #19669

Merged

This comment was marked as outdated.

Sign in to view

AlexWaygood force-pushed the alex/remove-tuples branch 2 times, most recently from 00b10f8 to 0fc1a93 Compare August 8, 2025 12:19

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch 2 times, most recently from 25a225c to 899fb87 Compare August 8, 2025 12:45

This comment was marked as outdated.

Sign in to view

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch from b16f9af to e8ceb16 Compare August 8, 2025 13:33

AlexWaygood mentioned this pull request Aug 8, 2025

[ty] Improve ability to solve TypeVars when they appear in unions #19829

Merged

AlexWaygood force-pushed the alex/remove-tuples branch 2 times, most recently from a24e949 to df1357f Compare August 8, 2025 16:51

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch from e8ceb16 to ce7ee42 Compare August 8, 2025 16:54

AlexWaygood marked this pull request as ready for review August 8, 2025 16:57

AlexWaygood requested review from carljm, dcreager and sharkdp as code owners August 8, 2025 16:57

AlexWaygood added the performance Potential performance improvement label Aug 8, 2025

This comment was marked as outdated.

Sign in to view

AlexWaygood force-pushed the alex/remove-tuples branch from df1357f to 1dc9a1b Compare August 8, 2025 22:22

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch from fbcb8e3 to c84f961 Compare August 8, 2025 22:25

AlexWaygood added 10 commits August 9, 2025 09:30

reduce diff

945a05a

add TODO tests for new false positives in primer

ac97b07

more tests

959f6d1

explicitly test single-valuedness of tuple subclasses

3136932

does this help perf

27691ef

does this help perf

d711bd8

cleanup function.md

e3aa7a5

micro-optimisations

8d620df

remove unnecessary call to find_legacy_typevars

deaa45e

Revert "[ty] Add Salsa caching to TupleType::to_class_type"

3436d5d

This reverts commit 252a603.

AlexWaygood force-pushed the alex/remove-tuples branch from 677d378 to 3436d5d Compare August 9, 2025 08:30

AlexWaygood added 7 commits August 9, 2025 09:36

Make NominalInstanceType class field private (thanks Claude)

4dffbaa

Make NominalInstanceType wrap an inner enum

4b27023

better variable names

17238a7

add back comment

484b64f

simplify

45c28fa

remove expect(dead_code) call

fa7aa09

fix severe perf regression on static-frame

3b39238

AlexWaygood force-pushed the alex/remove-tuples-instance-enum branch from 8c00cdb to 3b39238 Compare August 9, 2025 08:42

AlexWaygood commented Aug 10, 2025

View reviewed changes

docs

ae1e21b

AlexWaygood mentioned this pull request Aug 10, 2025

[ty] Add static-frame as a walltime benchmark #19844

Merged

simplify TupleType::to_class_type

dd260ae

AlexWaygood commented Aug 10, 2025

View reviewed changes

AlexWaygood mentioned this pull request Aug 10, 2025

Panic when building divergent tuple type for implicit instance attributes astral-sh/ty#837

Closed

AlexWaygood force-pushed the alex/remove-tuples branch from 3436d5d to da3bc43 Compare August 11, 2025 10:43

AlexWaygood closed this Aug 11, 2025

AlexWaygood deleted the alex/remove-tuples-instance-enum branch August 11, 2025 10:52

AlexWaygood mentioned this pull request Aug 12, 2025

[ty] support recursive type aliases #19805

Merged

[ty] Make NominalInstanceType internally wrap an enum #19811

[ty] Make NominalInstanceType internally wrap an enum #19811

Uh oh!

Conversation

AlexWaygood commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

codspeed-hq bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed WallTime Performance Report

Merging #19811 will improve performances by 63.76%

Summary

Benchmarks breakdown

Uh oh!

codspeed-hq bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #19811 will improve performances by 6.26%

Summary

Benchmarks breakdown

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

AlexWaygood commented Aug 8, 2025

Uh oh!

AlexWaygood commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

AlexWaygood Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Aug 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[ty] Make `NominalInstanceType` internally wrap an enum #19811

[ty] Make `NominalInstanceType` internally wrap an enum #19811

AlexWaygood commented Aug 7, 2025 •

edited

Loading

github-actions bot commented Aug 7, 2025 •

edited

Loading

github-actions bot commented Aug 7, 2025 •

edited

Loading

`mypy_primer` results

codspeed-hq bot commented Aug 7, 2025 •

edited

Loading

codspeed-hq bot commented Aug 7, 2025 •

edited

Loading

AlexWaygood commented Aug 8, 2025 •

edited

Loading