Skip to content

Remove computeNode and use new analyzer interfaces for library retrieval #1851

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 29 commits into from
Dec 4, 2018

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Nov 28, 2018

Refactors PackageGraph and Library construction to preserve the ResolvedLibraryResult objects from the analyzer, which contain the necessary AST nodes for Dartdoc ModelElements to be able to resolve source code and comment references. This PR is the culmination of a joint effort with @bwilkerson, @scheglov and others, many thanks for all their efforts.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Nov 28, 2018
@jcollins-g
Copy link
Contributor Author

Should be ready for review now with the publish of analyzer 0.33.5.

@jcollins-g
Copy link
Contributor Author

Fixed a race condition in library detection found by the integration tests, but waiting for Flutter tests to pass on a republish of analyzer's dependencies that include dev-9.4 in their SDK ranges.

Future<List<LibraryElement>> _parseLibraries(Set<String> files) async {
Iterable<LibraryElement> libraries = new Iterable.empty();
Future<Map<LibraryElement, ResolvedLibraryResult>> _parseLibraries(Set<String> files) async {
Map<LibraryElement, ResolvedLibraryResult> libraries = new Map();
Copy link
Member

Choose a reason for hiding this comment

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

Consider replacing "new Map()" with "{}".

@jcollins-g jcollins-g changed the title WIP: Remove computeNode and use new analyzer interfaces for library retrieval Remove computeNode and use new analyzer interfaces for library retrieval Nov 28, 2018
@jcollins-g
Copy link
Contributor Author

Another integration test failure for angular seems to be due to increased memory usage (since we're hanging on to many astNodes now this is not too surprising). I'll see if there's something simple I can find in the profiler to make things better while waiting for the next publish.

@jcollins-g
Copy link
Contributor Author

As expected, new analyzer publish fixes the version issue. But flutter is also blocked on the peak heap size problem. I have some threads to pursue on this.

@jcollins-g jcollins-g merged commit 9ad4ce8 into master Dec 4, 2018
@jcollins-g jcollins-g deleted the packageGraph-async+remove-computeNode branch December 4, 2018 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants