Speed up SCC dependency inference by JukkaL · Pull Request #18299 · python/mypy (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation4 Commits1 Checks18 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

JukkaL

Avoid redundant computation of frozenset(scc).

This helps with incremental type checking of torch, since it has a big SCC. In my measurements this speeds up incremental checking of -c "import torch" by about 11%.

@JukkaL

Avoid redundant computation of frozenset(scc).

This helps with incremental type checking of torch, since it has a big SCC. In my measurements this speeds up incremental checking of -c "import torch" by about 11%.

@github-actions GitHub Actions

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

cdce8p

@@ -57,7 +57,11 @@ def prepare_sccs(
sccs: list[set[T]], edges: dict[T, list[T]]
) -> dict[AbstractSet[T], set[AbstractSet[T]]]:
"""Use original edges to organize SCCs in a graph by dependencies between them."""
sccsmap = {v: frozenset(scc) for scc in sccs for v in scc}

Choose a reason for hiding this comment

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

If you wanted to keep the dict comprehension, you could use an assignment expression

sccsmap = {v: s for scc in sccs if (s := frozenset(scc)) is not None for v in scc} # type: ignore[redundant-expr]

It adds an additional if not None check for each scc that's always true but necessary to use :=.

Choose a reason for hiding this comment

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

Good point, but I think this version is clearer

hauntsaninja

@cdce8p

This helps with incremental type checking of torch, since it has a big SCC. In my measurements this speeds up incremental checking of -c "import torch" by about 11%.

To provide an additional data point. I'm seeing a ~10% improvement 🚀 on full runs (without cache) for Home Assistant. Just this PR and #18298.

Before 3:21
After 3:00