Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

avoid_web_libraries_in_flutter perf improvements: fix pubspec state caching #2642

Merged
merged 1 commit into from
May 12, 2021

Conversation

pq
Copy link
Contributor

@pq pq commented May 12, 2021

Context:

Before this change, we were parsing pubspecs for each and every compilation unit because our caching mechanism was faulty. This fixes that to limit parsing ideally to once per context root.

For reference, a typical benchmark run before this change was ~250ms. Because the code being benchmarked is not in a flutter package, with this change we're seeing no count. (See notes below.)

Notes:

  • benchmarks are currently not capturing time in registerNodeProcessors(...) so this lint is not measured by active benchmarks
  • when benchmarking flutter/packages/flutter_web_plugins (which is a flutter package and so is benchmarked using the current mechanism) we see an improvement from 20ms to ~0.
  • since this lint is actively in the flutter list, this improvement will affect LOTS of users

TL;DR: I think this approach is sound and a major improvement but benchmarking is lacking. It will be improved on the next analyzer push but we'll need to update what we're benchmarking against to get better results moving forward.

/cc @bwilkerson

/fyi @goderbauer

@google-cla google-cla bot added the cla: yes label May 12, 2021
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 93.898% when pulling 6bec20b on avoid_web_lib_check_fix into b448610 on master.

@pq pq changed the title improve pubspec state caching avoid_web_libraries_in_flutter perf improvements: fix pubspec state caching May 12, 2021
@bwilkerson
Copy link
Contributor

I agree, the changes look good.

We probably need to benchmark different rules against different code bases, given that some rules only apply to certain kinds of code bases.

@pq
Copy link
Contributor Author

pq commented May 12, 2021

We probably need to benchmark different rules against different code bases

Agreed. We've got a tracking issue for benchmarking in dart-lang/sdk#57576. It'd be great to bump this up in the coming quarters.

@pq pq requested a review from bwilkerson May 12, 2021 18:11
@pq pq merged commit 0cc2e29 into master May 12, 2021
@pq pq deleted the avoid_web_lib_check_fix branch May 12, 2021 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants