Skip to content

migration tool reanalyzes all parts in a library every time it analyzes one #40915

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.

Sign up for GitHub

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

Jump to bottom
Closed
jcollins-g opened this issue Mar 6, 2020 · 3 comments
Closed

migration tool reanalyzes all parts in a library every time it analyzes one #40915

jcollins-g opened this issue Mar 6, 2020 · 3 comments
Assignees
@jcollins-g
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). NNBD Issues related to NNBD Release P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@jcollins-g
Copy link
Contributor

In the debugger, I have seen AnalysisDriver._computeAnalysisResult calls with withUnit = true for every file in a multi-part library, which appears to force re-analysis of the full library. Exactly why we are doing this and what we might do instead is TBD. This is a big reason migration of web_ui is so slow.
@jcollins-g jcollins-g added legacy-area-analyzer Use area-devexp instead. analyzer-nnbd-migration NNBD Issues related to NNBD Release labels Mar 6, 2020
@bwilkerson
Copy link
Member

If you're using an AnalysisSession to perform analysis, then you can use getSourceKind to determine whether a file is a part. If you then use getResolvedLibrary for libraries and iterate over the already analyzed parts, you can keep track of which part's you didn't find a library for and analyze only those at the end (or signal an error if that's more appropriate).

@jcollins-g jcollins-g added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Mar 6, 2020
@jcollins-g jcollins-g self-assigned this Mar 9, 2020
@jcollins-g
Copy link
Contributor Author

jcollins-g commented Mar 9, 2020 •
edited
Loading

The pipeline migration uses is somewhat different. The dartfix (shared with dartdev) pipeline uses AbstractAnalysisServer.getResolvedUnit recursively over a file tree built with Resource.getChildren(). The way the resolved units are processed is going to be challenging to build a similar fix for as a lot of context information isn't in the right place for us to dedupe the getResolvedUnit calls.... at least, so far. Still looking.

@jcollins-g
Copy link
Contributor Author

https://dart-review.googlesource.com/c/sdk/+/138940 now has a proposed fix.

dart-bot pushed a commit that referenced this issue Mar 10, 2020
@jcollins-g
Do not repeatedly analyze a full library per part when dartfixing.
ace1d9b 
Migration performance improves 10x on web_ui with this CL.
Also fixes a heisenbug in the linter code where identical uniqueNames
for LintCodes could wind up in objects that compare differently.

Bug: #40915
Change-Id: Iddddfd7659f4bc724ddf21ebae41750a0ee51e74
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138940
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Janice Collins <[email protected]>
@stereotype441 stereotype441 added area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). and removed analyzer-nnbd-migration legacy-area-analyzer Use area-devexp instead. labels Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jcollins-g jcollins-g

Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). NNBD Issues related to NNBD Release P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stereotype441 @bwilkerson @jcollins-g