Skip to content

Performance improvements and refactoring #2259

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 1 commit into from
Jul 10, 2020
Merged

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Jul 9, 2020

When documenting the camera Flutter plugin, these changes reduce the time to document from 52 seconds to 51 seconds.

Much of this speedup likely came from memoizing Package.documentedWhere, which has a few expensive calls like linkToRemote and linkToUrl. Additionally ModelElement.locationPieces is now memoized.

The rest is a few categorical refactorings:

  • ModelElement.canonicalLibrary:
    • Refactor most of the complexity of ModelElement.canonicalLibrary into a new private method.
    • Don't compute topLevelElement until you need it.
    • Short circuit if definingLibrary.exportedInLibraries is null.
    • Reduce nesting
    • Create warning message only if it will be used.
  • Prefer for loops over Iterable.forEach.

When documenting the camera Flutter plugin, these changes reduce the time to
document from 52 seconds to 51 seconds.

Much of this speedup likely came from memoizing Package.documentedWhere, which
has a few expensive calls like `linkToRemote` and `linkToUrl`. Additionally
ModelElement.locationPieces is now memoized.

The rest is a few refactorings:

* ModelElement.canonicalLibrary:
    * Refactor most of the complexity of ModelElement.canonicalLibrary into a
      new private method.
    * Don't compute topLevelElement until you need it.
    * Short circuit if `definingLibrary.exportedInLibraries` is null.
    * Reduce nesting
    * Create warning message only if it will be used.
* Prefer for loops over Iterable.forEach.
@srawlins srawlins requested a review from scheglov July 9, 2020 23:26
@googlebot googlebot added the cla: yes Google CLA check succeeded. label Jul 9, 2020
Copy link
Contributor

@scheglov scheglov left a comment

Choose a reason for hiding this comment

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

52 to 51 is not a huge difference, but OTOH, I don't see any issues with the new code.

@srawlins
Copy link
Member Author

Yeah I was trying a bunch of little things, cleaning code, and none of it made it worse, and I think the code is more idiomatic and maintainable so, might as well cut and commit.

@srawlins srawlins merged commit d23db1b into dart-lang:master Jul 10, 2020
@srawlins srawlins deleted the perf-2 branch July 10, 2020 03:07
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.

None yet

3 participants