Skip to content

[ty] Restore IsNonTerminalCall shortcut optimization (again)#24185

Merged
sharkdp merged 3 commits intomainfrom
david/revert-shortcut-removal-again
Mar 26, 2026
Merged

[ty] Restore IsNonTerminalCall shortcut optimization (again)#24185
sharkdp merged 3 commits intomainfrom
david/revert-shortcut-removal-again

Conversation

@sharkdp
Copy link
Copy Markdown
Contributor

@sharkdp sharkdp commented Mar 25, 2026

Summary

I tried a few other things, but none of them worked, so for now, let's just revert a2c5af0 (again). Note that it's not an exact revert because we renamed NoReturn -> IsNonTerminalCall and inverted the logic in between. Also, I fixed the bug that was described in the original PR (#23378).

closes astral-sh/ty#3120

Note for reviewers: The only thing that's really new here is the benchmark, and the changes in the second commit (5348e47).

Test Plan

New benchmark

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Mar 25, 2026
@sharkdp sharkdp changed the title [ty] Restore IsNonTerminalCall shortcut optimization (again) [ty] Restore IsNonTerminalCall shortcut optimization (again) Mar 25, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made some attempts to minify this and/or change it to something that is easier in structure, but eventually decided to simply use the version from the issue.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 25, 2026

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 86.46%. The percentage of expected errors that received a diagnostic held steady at 80.68%. The number of fully passing files held steady at 67/132.

@sharkdp sharkdp added the performance Potential performance improvement label Mar 25, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 25, 2026

Memory usage report

Summary

Project Old New Diff Outcome
prefect 705.08MB 711.88MB +0.97% (6.81MB)
sphinx 262.42MB 264.18MB +0.67% (1.75MB)
trio 115.74MB 117.31MB +1.36% (1.58MB)
flake8 47.94MB 47.87MB -0.14% (66.74kB) ⬇️

Significant changes

Click to expand detailed breakdown

prefect

Name Old New Diff Outcome
infer_expression_types_impl 56.21MB 61.45MB +9.34% (5.25MB)
semantic_index 167.67MB 169.57MB +1.14% (1.91MB)
Expression 6.86MB 8.67MB +26.25% (1.80MB)
infer_definition_types 89.86MB 88.68MB -1.31% (1.18MB)
all_narrowing_constraints_for_expression 7.42MB 7.10MB -4.30% (326.86kB)
CallableType 1.73MB 2.02MB +16.82% (297.30kB)
infer_expression_type_impl 14.05MB 13.76MB -2.06% (297.11kB)
StaticClassLiteral<'db>::implicit_attribute_inner_ 10.11MB 9.90MB -2.11% (218.30kB)
all_negative_narrowing_constraints_for_expression 2.81MB 2.61MB -6.94% (199.65kB)
Type<'db>::member_lookup_with_policy_ 16.31MB 16.13MB -1.15% (192.38kB)
BoundMethodType<'db>::into_callable_type_ 186.47kB 309.23kB +65.84% (122.77kB)
Type<'db>::class_member_with_policy_ 17.77MB 17.65MB -0.67% (122.12kB)
loop_header_reachability 530.49kB 434.95kB -18.01% (95.54kB)
infer_scope_types_impl 53.65MB 53.71MB +0.12% (63.27kB)
Type<'db>::try_call_dunder_get_ 10.70MB 10.71MB +0.11% (11.53kB)
... 27 more

sphinx

Name Old New Diff Outcome
infer_expression_types_impl 19.69MB 21.35MB +8.44% (1.66MB)
semantic_index 59.72MB 60.42MB +1.18% (720.29kB)
Expression 2.50MB 3.17MB +26.76% (685.48kB)
infer_expression_type_impl 3.58MB 2.95MB -17.58% (644.00kB)
infer_definition_types 24.13MB 23.59MB -2.24% (553.20kB)
CallableType 943.89kB 1.09MB +18.37% (173.39kB)
all_narrowing_constraints_for_expression 2.48MB 2.34MB -5.82% (147.77kB)
loop_header_reachability 517.10kB 379.17kB -26.67% (137.93kB)
BoundMethodType<'db>::into_callable_type_ 176.19kB 278.74kB +58.20% (102.55kB)
all_negative_narrowing_constraints_for_expression 1.08MB 1.00MB -7.08% (78.27kB)
infer_scope_types_impl 15.52MB 15.53MB +0.12% (19.24kB)
UnionType<'db>::from_two_elements_ 1.39MB 1.38MB -0.68% (9.64kB)
infer_unpack_types 440.51kB 432.55kB -1.81% (7.96kB)
is_redundant_with_impl::interned_arguments 2.02MB 2.02MB -0.29% (6.10kB)
UnionType 1.24MB 1.24MB -0.41% (5.19kB)
... 24 more

trio

Name Old New Diff Outcome
infer_expression_types_impl 6.04MB 7.02MB +16.24% (1003.63kB)
Expression 1.09MB 1.41MB +28.66% (321.05kB)
semantic_index 29.29MB 29.58MB +0.99% (297.96kB)
infer_expression_type_impl 1.41MB 1.32MB -6.43% (92.48kB)
CallableType 481.56kB 563.86kB +17.09% (82.29kB)
all_narrowing_constraints_for_expression 626.97kB 589.09kB -6.04% (37.89kB)
BoundMethodType<'db>::into_callable_type_ 42.04kB 73.49kB +74.83% (31.45kB)
all_negative_narrowing_constraints_for_expression 210.86kB 183.22kB -13.11% (27.64kB)
infer_scope_types_impl 4.76MB 4.78MB +0.48% (23.50kB)
loop_header_reachability 137.42kB 133.07kB -3.16% (4.34kB)
infer_deferred_types 2.38MB 2.38MB +0.11% (2.73kB)
is_redundant_with_impl::interned_arguments 533.93kB 536.42kB +0.47% (2.49kB)
is_redundant_with_impl 469.91kB 472.30kB +0.51% (2.39kB)
Type<'db>::class_member_with_policy_ 1.99MB 1.99MB +0.10% (2.04kB)
Type<'db>::try_call_dunder_get_ 1.36MB 1.37MB +0.14% (1.96kB)
... 37 more

flake8

Name Old New Diff Outcome
infer_definition_types 1.92MB 1.82MB -4.83% (94.79kB) ⬇️
infer_expression_type_impl 198.88kB 141.81kB -28.70% (57.07kB) ⬇️
semantic_index 13.47MB 13.51MB +0.35% (48.51kB) ⬇️
infer_expression_types_impl 1.03MB 1.06MB +3.14% (33.08kB) ⬇️
Expression 334.62kB 365.70kB +9.29% (31.08kB) ⬇️
loop_header_reachability 43.10kB 13.36kB -69.01% (29.74kB) ⬇️
CallableType 144.28kB 167.98kB +16.42% (23.70kB) ⬇️
all_narrowing_constraints_for_expression 102.34kB 82.01kB -19.86% (20.33kB) ⬇️
BoundMethodType<'db>::into_callable_type_ 14.10kB 27.63kB +96.01% (13.54kB) ⬇️
infer_unpack_types 47.85kB 36.88kB -22.92% (10.97kB) ⬇️
all_negative_narrowing_constraints_for_expression 45.39kB 39.60kB -12.77% (5.80kB) ⬇️
infer_scope_types_impl 997.85kB 999.89kB +0.20% (2.04kB) ⬇️
Type<'db>::member_lookup_with_policy_ 488.41kB 488.42kB +0.00% (12.00B) ⬇️
StaticClassLiteral<'db>::implicit_attribute_inner_ 313.76kB 313.77kB +0.00% (12.00B) ⬇️

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 25, 2026

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-argument-type 1 0 0
invalid-assignment 1 0 0
invalid-return-type 0 0 1
type-assertion-failure 1 0 0
Total 3 0 1

Raw diff:

egglog-python (https://github.com/egraphs-good/egglog-python)
+ python/egglog/egraph.py:2139:24 error[invalid-argument-type] Argument to function `expr_action` is incorrect: Expected `BaseExpr`, found `(BaseExpr & ~Action) | (Fact & ~Action)`

hydpy (https://github.com/hydpy-dev/hydpy)
+ hydpy/core/devicetools.py:2425:9 error[type-assertion-failure] Type `Literal["inlets", "outlets", "observers", "receivers", "senders", "inputs", "outputs"]` is not equivalent to `Never`

pydantic (https://github.com/pydantic/pydantic)
- pydantic/v1/utils.py:613:16 error[invalid-return-type] Return type does not match returned value: expected `Mapping[int | str, Any]`, found `(AbstractSet[int | str] & Top[Mapping[Unknown, object]]) | Mapping[int | str, Any] | dict[int | str, ellipsis]`
+ pydantic/v1/utils.py:613:16 error[invalid-return-type] Return type does not match returned value: expected `Mapping[int | str, Any]`, found `(AbstractSet[int | str] & Top[Mapping[Unknown, object]]) | (Mapping[int | str, Any] & AbstractSet[object]) | (Mapping[int | str, Any] & ~AbstractSet[object]) | dict[int | str, ellipsis]`

trio (https://github.com/python-trio/trio)
+ src/trio/_core/_tests/test_run.py:2440:24 error[invalid-assignment] Object of type `Value[object] | Error` is not assignable to `Outcome[str] | None`

Full report with detailed diff (timing results)

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 25, 2026

Merging this PR will not alter performance

✅ 57 untouched benchmarks
🆕 1 new benchmark

Performance Changes

Mode Benchmark BASE HEAD Efficiency
🆕 Simulation ty_micro[typeis_narrowing] N/A 172.7 ms N/A

Comparing david/revert-shortcut-removal-again (44d7b1f) with main (7973118)

Open in CodSpeed

@sharkdp
Copy link
Copy Markdown
Contributor Author

sharkdp commented Mar 25, 2026

It's very unfortunate that I need to acknowledge a (small) performance regression on pandas in order to fix this performance regression in astral-sh/ty#3120, but I currently see no other way.

@sharkdp sharkdp marked this pull request as ready for review March 25, 2026 19:55
Copy link
Copy Markdown
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Too bad :/ Thanks for exploring!

@sharkdp
Copy link
Copy Markdown
Contributor Author

sharkdp commented Mar 25, 2026

the new benchmark is currently timing out after my last commit. need to take another look tomorrow.

@AlexWaygood
Copy link
Copy Markdown
Member

the new benchmark is currently timing out after my last commit. need to take another look tomorrow.

shows you wrote an effective regression benchmark!

@sharkdp sharkdp merged commit a84a58f into main Mar 26, 2026
49 checks passed
@sharkdp sharkdp deleted the david/revert-shortcut-removal-again branch March 26, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Potential performance improvement ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance regression in 0.0.24 with TypeIs narrowing + match on large Literal

3 participants