Repository navigation
Avoid quadratic rescans when ordering methods by subclassing - #22111
Conversation
This comment has been minimized.
This comment has been minimized.
c1de3e9 to
259c225
Compare
This comment has been minimized.
This comment has been minimized.
Hnasar
left a comment
Hnasar
left a comment
There was a problem hiding this comment.
Nice find and clever reimplementation.
| class A: | ||
| def a(self) -> None: ... | ||
|
|
||
| [targets __main__, __main__, __main__, __main__, __main__.D.N.n, __main__.f, __main__.A.a, __main__.C.c, __main__.B.b, __main__.D.d, __main__.E.e, __main__.D.d2] |
There was a problem hiding this comment.
Make sure that superclass methods are always processed before subclass methods
but
, __main__.E.e, __main__.D.d2]
Something seems off between these.
There was a problem hiding this comment.
You're right. The old code also unblocks subclasses after the superclass's first method group. I've clarified the docstring; this PR preserves that ordering.
There was a problem hiding this comment.
Actually, the old behavior is a bug. We should unblock subclasses after the last group. If this would be an easy fix, could you do this? Otherwise just leave it as is.
This comment has been minimized.
This comment has been minimized.
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks! Did you run some benchmarks with compiled mypy on some real-world code? For example mypy -c 'import torch' (with cold cache) may be a good benchmark for this.
| class A: | ||
| def a(self) -> None: ... | ||
|
|
||
| [targets __main__, __main__, __main__, __main__, __main__.D.N.n, __main__.f, __main__.A.a, __main__.C.c, __main__.B.b, __main__.D.d, __main__.E.e, __main__.D.d2] |
There was a problem hiding this comment.
Actually, the old behavior is a bug. We should unblock subclasses after the last group. If this would be an easy fix, could you do this? Otherwise just leave it as is.
|
I ran compiled mypy on these imports with a cold mypy cache. These are median times from five runs per version, comparing this PR with its parent:
Linux, Python 3.13.14, mypyc with There isn't a substantial end-to-end speedup on these imports. Transformers does execute about 2.3% fewer instructions, but the elapsed-time differences are small relative to the timing variation. I've put the superclass ordering fix in #22113. It waits for the last method group and fixes a case where mypy misses an incompatible assignment to an inherited attribute. |
…22113) A nested class can split its enclosing class's methods into several groups. Processing the first group currently unblocks subclasses while later base-class methods are still waiting, which can make mypy miss incompatible assignments to inherited attributes. Count the remaining groups for each class and only unblock subclasses after the last group has been processed. This fixes the existing ordering bug discussed in #22111 separately from that PR's performance change.
|
OK, thanks, I merged the other PR first, so there is a merge conflict now. Please feel free to rebase and merge. |
order_by_subclassing restarted its scan from the first group after
every class it processed, re-slicing info.mro[1:] on each check, so it
was quadratic in the number of blocked method groups. This shows up in
large import cycles where subclasses come before their bases: in a
profile of a big batch including transformers and torch it accounted
for about 6% of all CPU samples.
A blocked group now waits on the first unprocessed superclass in its
MRO and is retried from a heap once that class is processed, resuming
the MRO scan where it stopped. The output order is identical to the
old one: checked on 6000 random hierarchies (half with non-transitive
MROs, which a plugin can produce) and on every call while checking
transformers, pandas, sqlalchemy and mypy itself. The MRO scan is a
separate non-generator function because mypyc keeps generator locals
on the environment object, which made the loop about 2x slower.
Function compiled with mypyc, best of 3:
old new
4000 subclasses before their 4000 bases 338 ms 3.3 ms
random 4000-class hierarchy, shuffled 394 ms 3.1 ms
2000-deep chain, subclass first 2424 ms 14 ms
random 4000-class hierarchy, in order 2.1 ms 1.9 ms
2000-deep chain, in order 19 ms 14 ms
1000 class pairs, in order 0.40 ms 0.47 ms
Total time in the function on real code, interpreted:
transformers 0.377 s -> 0.069 s (slowest call 258 ms -> 15 ms)
pandas 0.063 s -> 0.030 s (slowest call 23 ms -> 6 ms)
sqlalchemy 0.008 s -> 0.004 s
mypy 0.009 s -> 0.004 s
End to end, a file with 4000 subclasses defined before their bases
now checks in 3.5 s instead of 6.0 s (interpreted).
6947810 to
064da82
Compare
|
Diff from mypy_primer, showing the effect of this PR on open source code: scipy-stubs (https://github.com/scipy/scipy-stubs)
- tests/misc/test_submodules_empty.pyi:3: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
|
Done but you need to merge as I don't have commit rights here. |
Track the first unprocessed superclass blocking each method group and retry the group when that superclass is processed. This removes the repeated scans from the beginning of the group list. Each retry resumes at the next MRO entry, avoiding repeated traversal and slicing of the same MRO.
Use a min-heap of original group indices to keep the existing processing order: the earliest available group still goes first, including when nested classes split an enclosing class's methods into several groups. Keep the MRO scan in a separate function so mypyc can use local variables for the loop instead of generator environment attributes.
Fixes #22110.