Skip to content

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 30, 2025

Update salsa to pull in salsa-rs/salsa#999 which fixes astral-sh/ty#1111

Test plan

Added mdtest

@MichaReiser MichaReiser added the ty Multi-file analysis & type inference label Sep 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

Diagnostic diff on typing conformance tests

Changes were detected when running ty on typing conformance tests
--- old-output.txt	2025-10-16 10:38:39.949432603 +0000
+++ new-output.txt	2025-10-16 10:38:43.284456279 +0000
@@ -1,5 +1,5 @@
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_type_statement.py`: `PEP695TypeAliasType < 'db >::value_type_(Id(d017)): execute: too many cycle iterations`
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(16443)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/ef9f932/src/function/execute.rs:402:17 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_type_statement.py`: `PEP695TypeAliasType < 'db >::value_type_(Id(d017)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/ef9f932/src/function/execute.rs:402:17 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(16443)): execute: too many cycle iterations`
 _directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
 _directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
 _directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__`

@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

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 force-pushed the micha/fixpoint-scc branch 2 times, most recently from 9ea24ba to 7e22d7b Compare October 10, 2025 11:15
@MichaReiser
Copy link
Member Author

Hmm, this does seem to regress perf for quiet a few projects (1-4%)

@MichaReiser MichaReiser force-pushed the micha/fixpoint-scc branch 3 times, most recently from 9dd4a83 to 197d2d1 Compare October 10, 2025 13:37
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 10, 2025

CodSpeed Performance Report

Merging #20645 will degrade performances by 5.71%

Comparing micha/fixpoint-scc (598b8a7) with main (c9dfb51)

Summary

❌ 1 (👁 1) regression
✅ 50 untouched

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
👁 WallTime medium[pandas] 31.4 s 33.3 s -5.71%

@MichaReiser
Copy link
Member Author

Okay, I think I now have the right amount of inline attributes in salsa so that this no longer regresses performance

@MichaReiser MichaReiser force-pushed the micha/fixpoint-scc branch 2 times, most recently from 76cb068 to 94d5cf2 Compare October 16, 2025 10:35
@MichaReiser MichaReiser changed the title Update Salsa Fix run-away for self-referential implicit-attribute Oct 16, 2025
@MichaReiser MichaReiser changed the title Fix run-away for self-referential implicit-attribute Fix run-away for mutually referential instance attributes Oct 16, 2025
@MichaReiser MichaReiser marked this pull request as ready for review October 16, 2025 10:55
spark # too many iterations (in `exported_names` query)
steam.py # hangs (single threaded)
spark # too many iterations (in `exported_names` query), `should not be able to access instance member `spark` of type variable IndexOpsLike@astype in inferable position`
steam.py # dependency graph cycle when querying TypeVarInstance < 'db >::lazy_default_(Id(2e007)), set cycle_fn/cycle_initial to fixpoint iterate.
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be easy to fix but let's do this in a separate PR

@MichaReiser
Copy link
Member Author

Hmm, the small performance regression (except pandas) is back. I suspect it's just due to differences in inlining, now that we switched to LTO fat? I don't think it's worth narrowing this down right now

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.

Thank you!!

Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Congratulations for fixing this — thank you so much!

@MichaReiser MichaReiser merged commit 5fb1423 into main Oct 16, 2025
41 checks passed
@MichaReiser MichaReiser deleted the micha/fixpoint-scc branch October 16, 2025 11:24
dcreager added a commit that referenced this pull request Oct 16, 2025
…rable

* origin/main:
  Don't use codspeed or depot runners in CI jobs on forks (#20894)
  [ty] cache Type::is_redundant_with (#20477)
  Fix run-away for mutually referential instance attributes (#20645)
  [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912)
  [ty] Use field-specifier return type as the default type for the field (#20915)
  [ty] Do not assume that `field`s have a default value (#20914)
  [ty] Fix match pattern value narrowing to use equality semantics (#20882)
  Update setup instructions for Zed 0.208.0+ (#20902)
  Move TOML indent size config (#20905)
  [syntax-errors]: implement F702 as semantic syntax error (#20869)
  [ty] Heterogeneous unpacking support for unions (#20377)
  [ty] refactor `Place` (#20871)
  Auto-accept snapshot changes as part of typeshed-sync PRs (#20892)
  [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551)
  [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508)
  [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660)
  Update parser snapshots (#20893)
  Fix syntax error false positives for escapes and quotes in f-strings (#20867)
dcreager added a commit that referenced this pull request Oct 16, 2025
…nt-sets

* dcreager/non-non-inferable:
  Don't use codspeed or depot runners in CI jobs on forks (#20894)
  [ty] cache Type::is_redundant_with (#20477)
  Fix run-away for mutually referential instance attributes (#20645)
  [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912)
  [ty] Use field-specifier return type as the default type for the field (#20915)
  [ty] Do not assume that `field`s have a default value (#20914)
  [ty] Fix match pattern value narrowing to use equality semantics (#20882)
  Update setup instructions for Zed 0.208.0+ (#20902)
  Move TOML indent size config (#20905)
  [syntax-errors]: implement F702 as semantic syntax error (#20869)
  [ty] Heterogeneous unpacking support for unions (#20377)
  [ty] refactor `Place` (#20871)
  Auto-accept snapshot changes as part of typeshed-sync PRs (#20892)
  [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551)
  [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508)
  [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660)
  Update parser snapshots (#20893)
  Fix syntax error false positives for escapes and quotes in f-strings (#20867)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Excessive runtime for mutually referential instance attributes

4 participants