Skip to content

Comments

[ty] Track heap usage of salsa structs#19790

Merged
MichaReiser merged 7 commits intoastral-sh:mainfrom
ibraheemdev:ibraheem/structs-memory-usage
Aug 12, 2025
Merged

[ty] Track heap usage of salsa structs#19790
MichaReiser merged 7 commits intoastral-sh:mainfrom
ibraheemdev:ibraheem/structs-memory-usage

Conversation

@ibraheemdev
Copy link
Member

Summary

This is the last big remaining item for our Salsa memory reports.

@ibraheemdev ibraheemdev changed the title Track heap usage of salsa structs [ty] Track heap usage of salsa structs Aug 6, 2025
@ibraheemdev ibraheemdev added ty Multi-file analysis & type inference internal An internal refactor or improvement labels Aug 6, 2025
@ibraheemdev ibraheemdev force-pushed the ibraheem/structs-memory-usage branch from 8186e3f to cc02bf8 Compare August 6, 2025 19:47

for (query_fn, memo) in memos {
let size_of_fields =
memo.size_of_fields() + memo.heap_size_of_fields().unwrap_or(0);
Copy link
Member Author

Choose a reason for hiding this comment

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

Would be nice if there was a total_size_of_fields helper.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 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.

@ibraheemdev ibraheemdev force-pushed the ibraheem/structs-memory-usage branch from cc02bf8 to 6b7d931 Compare August 6, 2025 20:35
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

mypy_primer results

No ecosystem changes detected ✅

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

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

prefect (https://github.com/PrefectHQ/prefect)
-     struct fields = ~33MB
+     struct fields = ~40MB

@AlexWaygood AlexWaygood removed their request for review August 6, 2025 20:57
Copy link
Member

@MichaReiser MichaReiser 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, this is great.

Did you run ty on any large project with memory reporting enabled? Any interesting (large structs) finds?

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to your changes: Would it make sense for the CI job to show something closer to FULL (while still bucketing the numbers)? I find the current output a good indicator but it isn't actionable because all I know is that memory usage increased or decreased. It might be more helpful if we can pin point the change to specific queries.

@ibraheemdev ibraheemdev force-pushed the ibraheem/structs-memory-usage branch 2 times, most recently from 6879f08 to 810df06 Compare August 11, 2025 17:23
@MichaReiser
Copy link
Member

MichaReiser commented Aug 12, 2025

I'll merge this because I think it will make @AlexWaygood's life a bit easier and I think all review feedback is addressed too (I just did a merge and added a few more get size implementations)

@MichaReiser MichaReiser merged commit f34b65b into astral-sh:main Aug 12, 2025
37 checks passed
dcreager added a commit that referenced this pull request Aug 12, 2025
* main:
  Don't cache files with diagnostics (#19869)
  [ty] support recursive type aliases (#19805)
  [ty] Remove unsafe `salsa::Update` implementations in `tuple.rs` (#19880)
  [ty] Function argument inlay hints (#19269)
  [ty] Remove Salsa interning for `TypedDictType` (#19879)
  Fix `lint.future-annotations` link (#19876)
  [ty] Reduce memory usage of `TupleSpec` and `TupleType` (#19872)
  [ty] Track heap usage of salsa structs (#19790)
  Update salsa to pull in tracked struct changes (#19843)
  [ty] simplify CycleDetector::visit signature (#19873)
  [ty] use interior mutability in type visitors (#19871)
  [ty] Fix tool name is None when no ty path is given in ty_benchmark (#19870)
  [ty] Remove `Type::Tuple` (#19669)
  [ty] Short circuit `ReachabilityConstraints::analyze_single` for dynamic types (#19867)
dcreager added a commit that referenced this pull request Aug 13, 2025
…aints

* dcreager/inferrable: (65 commits)
  this was right after all
  mark typevars inferrable as we go
  fix tests
  fix inference of constrained typevars
  [ty] Add precise inference for indexing, slicing and unpacking `NamedTuple` instances (#19560)
  Add rule code to GitLab description (#19896)
  [ty] render docstrings in hover (#19882)
  [ty] simplify return type of place_from_declarations (#19884)
  [ty] Various minor cleanups to tuple internals (#19891)
  [ty] Improve `sys.version_info` special casing (#19894)
  Don't cache files with diagnostics (#19869)
  [ty] support recursive type aliases (#19805)
  [ty] Remove unsafe `salsa::Update` implementations in `tuple.rs` (#19880)
  [ty] Function argument inlay hints (#19269)
  [ty] Remove Salsa interning for `TypedDictType` (#19879)
  Fix `lint.future-annotations` link (#19876)
  [ty] Reduce memory usage of `TupleSpec` and `TupleType` (#19872)
  [ty] Track heap usage of salsa structs (#19790)
  Update salsa to pull in tracked struct changes (#19843)
  [ty] simplify CycleDetector::visit signature (#19873)
  ...
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