Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 86.61%. The percentage of expected errors that received a diagnostic held steady at 81.56%. The number of fully passing files held steady at 70/132. |
Memory usage reportMemory usage unchanged ✅ |
| /// Materialization is the only mapping mode that needs to visit the same type under two different | ||
| /// mappings within a single recursive call chain (`Top` and `Bottom`). Keep separate cycle caches | ||
| /// for those modes so invariant checks can safely reuse one visitor. |
There was a problem hiding this comment.
I'm not actually sure we need a cache for apply_type_mapping at all. I don't think it's nearly as effective an optimisation as it is for has_relation_to, and as you point out it's not very robust. I've been wondering for a bit if we should try ripping out the caching infrastructure from the CycleDetector and make the cycle detectors in types/relation.rs newtype wrappers around CycleDetector that add caching
There was a problem hiding this comment.
#24017 is also a way of getting rid of the problematica caching from apply_type_mapping, which works from the opposite premise of what I just said above (that apply_type_mapping is the "special" method that needs to be treated specially), but I've since come to believe that the special method is probably really has_relation_to; I'm not sure any other method that has cycle detectors really needs the caching infrastructure
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
0 | 40 | 0 |
invalid-return-type |
0 | 1 | 0 |
| Total | 0 | 41 | 0 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
Merging this PR will degrade performance by 16.32%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | static_frame |
21.4 s | 22.5 s | -5.11% |
| ❌ | WallTime | pandas |
68.8 s | 72.7 s | -5.35% |
| ❌ | WallTime | altair |
5.1 s | 5.3 s | -4.77% |
| ❌ | Simulation | ty_micro[typeis_narrowing] |
173.2 ms | 191 ms | -9.32% |
| ❌ | Simulation | ty_micro[large_union_narrowing] |
497.8 ms | 530.7 ms | -6.21% |
| ❌ | Simulation | ty_micro[many_enum_members_2] |
172.6 ms | 206.3 ms | -16.32% |
| ❌ | WallTime | pydantic |
7.4 s | 7.8 s | -4.99% |
| ❌ | WallTime | colour_science |
58.6 s | 61.8 s | -5.21% |
Comparing crmarsh/ty-recursive-alias-stack-overflow (9c476e2) with main (e871de4)
Footnotes
-
60 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
2 participants
No description provided.