Skip to content

[ty] Use a simpler ordering for BDD variables#22777

Merged
dcreager merged 1 commit intomainfrom
dcreager/simpler-ordering
Jan 21, 2026
Merged

[ty] Use a simpler ordering for BDD variables#22777
dcreager merged 1 commit intomainfrom
dcreager/simpler-ordering

Conversation

@dcreager
Copy link
Member

This PR updates the ordering that we use for the BDD variables in our constraint set representation. If we only care about correctness, we can choose any ordering that we want, as long as it's consistent. However, different orderings can have very different performance characteristics. Many BDD libraries attempt to reorder variables on the fly while building and working with BDDs. We don't do that, but we have tried to make some simple choices that have clear wins.

In particular, we now use the IDs that salsa assigns to each constraint as it is created. This tends to ensure that constraints that are close to each other in the source are also close to each other in the BDD structure.

As an optimization, we also reverse this ordering, so that constraints that appear earlier in the source appear "lower" (closer to the terminal nodes) in the BDD. Since we build up BDDs by combining smaller BDDs (which will have been constructed from expressions earlier in the source), this tends to minimize the amount of "node shuffling" that we have to do when combining BDDs.

Previously, we tried to be more clever — for instance, by comparing the typevars of each constraint first, in an attempt to keep all of the constraints for a single typevar adjacent in the BDD structure. However, this proved to be counterproductive; we've found empirically that we get smaller BDDs with an ordering that is more aligned with source order.

@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 20, 2026

Typing conformance results

No changes detected ✅

@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 20, 2026

mypy_primer results

Changes were detected when running on open source projects
attrs (https://github.com/python-attrs/attrs)
- tests/test_validators.py:1257:20: error[invalid-argument-type] Argument is incorrect: Expected `int | float`, found `Literal["spam"]`
+ tests/test_validators.py:1257:20: error[invalid-argument-type] Argument is incorrect: Expected `float | int`, found `Literal["spam"]`

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`

Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/dependencies/data.py:347:12: error[invalid-return-type] Return type does not match returned value: expected `_T@cached_inject`, found `Coroutine[Any, Any, _T@cached_inject | Coroutine[Any, Any, _T@cached_inject]] | _T@cached_inject`
+ tanjun/dependencies/data.py:347:12: error[invalid-return-type] Return type does not match returned value: expected `_T@cached_inject`, found `_T@cached_inject | Coroutine[Any, Any, _T@cached_inject | Coroutine[Any, Any, _T@cached_inject]]`

pwndbg (https://github.com/pwndbg/pwndbg)
- pwndbg/aglib/kernel/__init__.py:218:20: error[unsupported-operator] Operator `+` is not supported between objects of type `Unknown | None` and `Literal[1]`
+ pwndbg/aglib/kernel/__init__.py:218:20: error[unsupported-operator] Operator `+` is not supported between objects of type `None | Unknown` and `Literal[1]`
- pwndbg/aglib/kernel/__init__.py:224:22: error[unsupported-operator] Operator `+` is not supported between objects of type `Unknown | None` and `int`
+ pwndbg/aglib/kernel/__init__.py:224:22: error[unsupported-operator] Operator `+` is not supported between objects of type `None | Unknown` and `int`

static-frame (https://github.com/static-frame/static-frame)
- 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/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]`

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ tests/frame/test_groupby.py:229:15: error[type-assertion-failure] Type `Series[Any]` does not match asserted type `Series[str | bytes | int | ... omitted 12 union elements]`
+ tests/frame/test_groupby.py:625:15: error[type-assertion-failure] Type `Series[Any]` does not match asserted type `Series[str | bytes | int | ... omitted 12 union elements]`
- Found 4454 diagnostics
+ Found 4456 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 2058 diagnostics
+ Found 2059 diagnostics

Memory usage changes were detected when running on open source projects
trio (https://github.com/python-trio/trio)
-     struct fields = ~12MB
+     struct fields = ~11MB

@dcreager dcreager added internal An internal refactor or improvement ty Multi-file analysis & type inference ecosystem-analyzer labels Jan 20, 2026
@astral-sh-bot
Copy link

astral-sh-bot bot commented Jan 20, 2026

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-argument-type 2 2 4
invalid-await 0 2 6
invalid-assignment 0 0 5
invalid-return-type 1 0 3
possibly-missing-attribute 0 3 1
unused-ignore-comment 1 2 0
unresolved-attribute 0 0 2
unsupported-operator 0 0 2
Total 4 9 23

Full report with detailed diff (timing results)

@dcreager dcreager merged commit 6347407 into main Jan 21, 2026
52 checks passed
@dcreager dcreager deleted the dcreager/simpler-ordering branch January 21, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecosystem-analyzer 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.

3 participants

Comments