Reduce use of generators in finding files to document #3961
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The short story of this change is that we're refactoring the "file-finding" code so that
_findFilesToDocumentInPackageis no longerasync*, and_listDiris no longersync*. Lazy generators like these are nice when the full sequence of elements may not be generated. However, in this code, we definitely iterate over every element returned by these two functions, andsync*is known to be slow, and a package set may be thousands of files.The longer story has more details:
findFilesToDocumentInPackagewas async because of the code that finds a package config, and that code is only needed when theincludeDependenciesparameter istrue. The function only has two callers, one of which always passedincludeDependencies: false._findPackagesToDocument, only called by_getFilesToDocument. Now_findFilesToDocumentInPackagedoesn't need to be async, or a generator.getFilesToDocument: This function can actually be split into two cases: are we documenting the SDK, or not?auto-include-dependenciesflag, as it is never used when documenting the SDK. We also don't need to consider the deprecatedinclude-externalsflag, as it is also never used when documenting the SDK. So we return a very simple calculation in this case. When documenting the SDK, the code is now simpler and does less work._discoverLibrariesis only called once, we don't need to pass in theaddLibrarycallback. Instead,_discoverLibrariescan directly calluninitializedPackageGraph.addLibraryToGraph.I benchmarked this both documenting the SDK and googleapis package. There was no visible difference in time-to-document or memory usage when documenting the SDK. For the googleapis package, there was a -1% time-to-document, and -0.5% memory usage, which might well be within the error margins. 🤷
Contribution guidelines:
dart format.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.