Skip to content

[ty] Pass slice to specialize#22421

Merged
MichaReiser merged 2 commits intomainfrom
micha/specialize-slice
Jan 9, 2026
Merged

[ty] Pass slice to specialize#22421
MichaReiser merged 2 commits intomainfrom
micha/specialize-slice

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jan 6, 2026

Summary

We call specialize or to_specialized_class in many places with a fixed number of specializations, and probably often with the very same arguments.

Today's specialize implementation always collectes all specialization
into a Box<[T]> to perform the Salsa lookup. However, that Box is immediately discarded if the value was interned before.

This PR changes specialize to take a Cow<[T]>, allowing allocation-free lookups if the number of specialization-items are known at compile time (or we already have a slice). Call-sites where this isn't the case now have to collect to a Vec, but that's no different to what we do today, except that the cost is visible at the call site.

Codspeed shows a 1-3% perf improvement on different benchmarks.

Requires salsa-rs/salsa#1054

Test Plan

cargo test

@MichaReiser MichaReiser added internal An internal refactor or improvement ty Multi-file analysis & type inference labels Jan 6, 2026
@MichaReiser MichaReiser force-pushed the micha/specialize-slice branch from 396c22d to ae7ce38 Compare January 6, 2026 17:47
@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 6, 2026

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 6, 2026

mypy_primer results

Changes were detected when running on open source projects
tornado (https://github.com/tornadoweb/tornado)
- tornado/gen.py:255:62: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `None | Awaitable[Unknown] | list[Awaitable[Unknown]] | dict[Any, Awaitable[Unknown]] | Future[Unknown]`, found `_T@next | _VT@next | _T@next`
+ tornado/gen.py:255:62: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `None | Awaitable[Unknown] | list[Awaitable[Unknown]] | dict[Any, Awaitable[Unknown]] | Future[Unknown]`, found `_T@next | _T@next | _VT@next`

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
- src/scikit_build_core/build/wheel.py:98:20: error[no-matching-overload] No overload of bound method `__init__` matches arguments
- Found 48 diagnostics
+ Found 47 diagnostics

static-frame (https://github.com/static-frame/static-frame)
- static_frame/core/bus.py:671:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemLocReduces[Bus[Any], object_]`, found `InterGetItemLocReduces[Bus[Any] | Bottom[Series[Any, Any]] | ndarray[Never, Never] | ... omitted 6 union elements, object_]`
- static_frame/core/bus.py:675:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemILocReduces[Bus[Any], object_]`, found `InterGetItemILocReduces[Bus[Any] | Bottom[Index[Any]] | TypeBlocks | ... omitted 6 union elements, object_ | Self@iloc]`
+ static_frame/core/bus.py:675:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemILocReduces[Bus[Any], object_]`, found `InterGetItemILocReduces[Self@iloc | Bus[Any], object_ | Self@iloc]`
- static_frame/core/index.py:580:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemLocReduces[TVContainer_co@loc, TVDtype@Index]`, found `InterGetItemLocReduces[Any | Bottom[Series[Any, Any]], TVDtype@Index]`
+ static_frame/core/index.py:580:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemLocReduces[TVContainer_co@loc, TVDtype@Index]`, found `InterGetItemLocReduces[Bottom[Series[Any, Any]] | Any, TVDtype@Index]`
- static_frame/core/series.py:772:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemILocReduces[Series[Any, Any], TVDtype@Series]`, found `InterGetItemILocReduces[Series[Any, Any] | ndarray[Never, Never] | TypeBlocks | ... omitted 6 union elements, TVDtype@Series]`
+ static_frame/core/series.py:772:16: error[invalid-return-type] Return type does not match returned value: expected `InterGetItemILocReduces[Series[Any, Any], TVDtype@Series]`, found `InterGetItemILocReduces[Series[Any, Any] | Bottom[Index[Any]] | TypeBlocks | ... omitted 6 union elements, TVDtype@Series]`
- Found 1838 diagnostics
+ Found 1837 diagnostics

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/chain/decoding/tools.py:96:44: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- rotkehlchen/chain/decoding/tools.py:97:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `BTCAddress | ChecksumAddress | SubstrateAddress | SolanaAddress`, found `A@BaseDecoderTools`
+ rotkehlchen/chain/decoding/tools.py:99:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `Sequence[A@BaseDecoderTools]`, found `Unknown | tuple[BTCAddress, ...] | tuple[ChecksumAddress, ...] | tuple[SubstrateAddress, ...] | tuple[SolanaAddress, ...]`
- rotkehlchen/chain/decoding/tools.py:98:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `BTCAddress | ChecksumAddress | SubstrateAddress | SolanaAddress | None`, found `A@BaseDecoderTools | None`
+ rotkehlchen/chain/decoding/tools.py:100:62: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 2100 diagnostics
+ Found 2101 diagnostics

core (https://github.com/home-assistant/core)
- homeassistant/util/variance.py:47:12: error[invalid-return-type] Return type does not match returned value: expected `(**_P@ignore_variance) -> _R@ignore_variance`, found `_Wrapped[_P@ignore_variance, _R@ignore_variance | int | float | datetime, _P@ignore_variance, _R@ignore_variance | int | float | datetime]`
- Found 14492 diagnostics
+ Found 14491 diagnostics

No memory usage changes detected ✅

@MichaReiser MichaReiser force-pushed the micha/specialize-slice branch from ae7ce38 to cffe8de Compare January 6, 2026 17:50
@MichaReiser MichaReiser force-pushed the micha/specialize-slice branch 3 times, most recently from 797d90e to 83e0cc5 Compare January 7, 2026 08:23
@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 7, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser marked this pull request as ready for review January 7, 2026 08:50
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

This is great! We could probably also e.g. adjust the signature of Type::string_literal() to take a Cow<CompactString> instead of &str, if that salsa change lands.

@MichaReiser
Copy link
Member Author

This is great! We could probably also e.g. adjust the signature of Type::string_literal() to take a Cow instead of &str, if that salsa change lands.

I think that would require one more Lookup implementation on the salsa side

@MichaReiser MichaReiser force-pushed the micha/specialize-slice branch from 83e0cc5 to d2d4555 Compare January 9, 2026 08:28
@MichaReiser
Copy link
Member Author

This is great! We could probably also e.g. adjust the signature of Type::string_literal() to take a Cow instead of &str, if that salsa change lands.

I didn't end up doing this as part of this PR but the necessary changes are in place in the Salsa version that we use now

@MichaReiser MichaReiser merged commit ba5dd58 into main Jan 9, 2026
48 checks passed
@MichaReiser MichaReiser deleted the micha/specialize-slice branch January 9, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants