search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.2k stars 1.57k forks source link

Importing analysis_options.yaml greatly reduces analysis performance #35637

Closed dnfield closed 3 weeks ago

dnfield commented 5 years ago

see https://github.com/flutter/flutter/pull/26244

Goal of that PR is to reduce duplicate of analysis_options.yaml across multiple repos by moving it to a package and importing it. It does so by moving the options to a package, and in various places using include: package:....

It caused our analysis benchmarks to jump from 60s to 90s: https://flutter-dashboard.appspot.com/benchmarks.html

dnfield commented 5 years ago

/cc @Hixie @devoncarew

davidmorgan commented 5 years ago

Hi Dan,

We're using imports with package:pedantic, so I'd like to understand what happened with the benchmark here. I experimented with imports and both dartanalyzer on the command line in IntelliJ, and couldn't repro: https://github.com/dart-lang/pedantic/pull/22#issuecomment-491603852 ... is there any chance you could point me to more details on what the benchmark is doing please? If there's an easy way for me to repro locally that'd be great...

Thanks!

Morgan

dnfield commented 5 years ago

When we tried to switch Flutter's analysis options files to package references and relative file references, it made analysis take about 1.5x longer. The benchmark basically ran dartanalyzer 3 times nad measured the average time.

dnfield commented 5 years ago

It's possible that in the time since filing this the issue has been fixed - we could try reapplying the move to flutter and see if the benchmark still regresses.

davidmorgan commented 5 years ago

Yes, I think it's worth checking.

Or if there's a way to just run the benchmark offline we could do that?

dnfield commented 5 years ago

There is - https://github.com/flutter/flutter/blob/master/dev/devicelab/lib/tasks/analysis.dart

devoncarew commented 5 years ago

@davidmorgan, the benchmark is largely testing the same thing as flutter analyze --flutter-repo --benchmark.

devoncarew commented 5 years ago

You might want to run flutter update-packages first, if you haven't done that in your flutter checkout in a while.

joshprzybyszewski-wf commented 3 years ago

@dnfield , based on this comment, would it be possible and worth the effort to look at re-introducing the change to "reduce duplicate of analysis_options.yaml" in the flutter repo? I'm curious if the significant changes will positively impact a public repo on the latest versions of dart.

dnfield commented 3 years ago

We could try and then see what it does to the benchmark(s). However, I think the priority is not quite as high anymore with the flutter_lints package, and it's mainly something that would help the flutter specific repositories rather than flutter applications.

srawlins commented 3 weeks ago

I'm marking this as stale; analysis options were greatly refactored for Dart 3.4 and 3.5. If this is observed anew, we can look into it.