Skip to content

Backport "Make CheckUnused not slow." to LTS #21101

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 6, 2024
Merged

Backport "Make CheckUnused not slow." to LTS #21101

merged 10 commits into from
Jul 6, 2024

Conversation

WojciechMazur
Copy link
Contributor

Backports #20321 to the LTS branch.

PR submitted by the release tooling.
[skip ci]

sjrd added 10 commits July 6, 2024 17:18
It is used for every single tree in `CheckUnused`, so this is worth
it.

[Cherry-picked 69664f7]
It is not efficient when the results are always used exactly once.

[Cherry-picked 0bd4b15]
`Tree`s have structural equality. Even if `==` should be able to
exit quickly either because of `eq` or an early difference, sets
systematically call `hashCode`, which is going to recurse into the
entire structure.

[Cherry-picked a55ee4d]
It is pointless to sort a list before converting it into a Set.

[Cherry-picked 701d69f]
Instead of dealing with entire `tpd.Import`s at the end of the
scope, we eagerly flatten them into individual `ImportSelector`s.
We store them along with some data, including a mutable flag for
whether a selector has been used.

This allows to dramatically simplify `isInImport`, as well as more
aggressively cache the resolution of selectors. We also get rid of
the `IdentityHashMap`.

The algorithm is still O(n*m) where n is the number of imports in
a scope, and m the number of references found in that scope. It is
not entirely clear to me whether the previous logic was already
O(n*m) or worse (it may have included an additional p factor for
the number of possible selections from a given qualifier).

Regardless, it is already quite a bit faster than before, thanks
to smaller constant factors.

[Cherry-picked 975df4a]
That test does not rely on any information dependent on the import
selectors. It only relies on information at the use site. Eagerly
checking it means we do not put as many symbols into the
`usedInScope` set, which is good because it is one of the
complexity factors of the unused-imports analysis.

[Cherry-picked 8553bfc]
Base automatically changed from lts-20337 to lts-3.3 July 6, 2024 21:54
@WojciechMazur
Copy link
Contributor Author

No regressions detected in the community build up to lts-20279.

Reference

@WojciechMazur WojciechMazur merged commit 95f012b into lts-3.3 Jul 6, 2024
19 checks passed
@WojciechMazur WojciechMazur deleted the lts-20321 branch July 6, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants