[DO_NOT_MERGE] research: CLI performance issue profiling

[roman-petrov]

I started integrating the project into our codebase and found that CLI runs extremely slow compared to dart analyze command.

dart analyze command takes about 3-5 seconds on our codebase, dart_code_metrics CLI runs about two minutes.

I am not sure if this is some environment configuration or maybe Windows issue. On my machine results of this simple profiling code are:

contextForMs: 76, getResolvedUnitMs: 115290, runAnalysisforFileMs: 128.

So analysisContext.currentSession.getResolvedUnit method call seems to be extremely slow (in another performance test I made I see that it might even take several seconds to complete for just one file). At the moment I do not know how to speed up this...

@incendial, @dkrutskikh - can you please check (when you have time) if this issue can be reproduced on your machines and if we actually have serious performance issue here?

[codecov]

Codecov Report

Merging #344 (5be2f2e) into master (f7f5636) will increase coverage by 0.14%.
The diff coverage is 100.00%.

❗ Current head 5be2f2e differs from pull request most recent head aef9238. Consider uploading reports for the commit aef9238 to get more accurate results

@@            Coverage Diff             @@
##           master     #344      +/-   ##
==========================================
+ Coverage   77.61%   77.76%   +0.14%     
==========================================
  Files         143      143              
  Lines        3302     3324      +22     
==========================================
+ Hits         2563     2585      +22     
  Misses        739      739              

Impacted Files	Coverage Δ
lib/src/analyzers/lint_analyzer/lint_analyzer.dart	93.29% <100.00%> (+0.43%)	⬆️
...oid_returning_widgets/avoid_returning_widgets.dart	100.00% <100.00%> (ø)
...es_list/avoid_returning_widgets/config_parser.dart	100.00% <100.00%> (ø)
...es/rules_list/avoid_returning_widgets/visitor.dart	100.00% <100.00%> (ø)
...t_annotation_arguments_ordering/config_parser.dart	100.00% <100.00%> (ø)
...ules/rules_list/member_ordering/config_parser.dart	100.00% <100.00%> (ø)
...s_list/member_ordering_extended/config_parser.dart	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7f5636...aef9238. Read the comment docs.

[dkrutskikh]

@roman-petrov please try to migrate on getResolvedUnit2

[roman-petrov]

@dkrutskikh, I already tried to migrate to getResolvedUnit2 and saw no any difference in terms of performance. I think we should check if this performance issue exists and can be easily reproduced first and then decide if it possible to fix it...

[incendial]

@roman-petrov the problem is that this is the only known way to get info about types. We can use currentSession.getParsedUnit2 instead and it will improve the performance a lot (the result of running tests is ~5 times faster 00:43 +205 -6: Some tests failed. ). But for those 6 cases we need a type info.

I will take a closer look later.

[roman-petrov]

@incendial, thank you for the clarification. I understand that this not an easy to solve issue. But currently due to this issue CLI speed might be not acceptable in relatively large code base...

I see several ways to improve this:

• Use getParsedUnit2 for cases/rules we don't need type info.
• Investigate how analysis works in Dart analyzer (through server): it has good performance, but I am not sure it uses type information
• Cooperate with DartSDK team to find out how we can improve analysis performance (maybe there is some different API we should use for our scenario)

It seems getResolvedUnit2 in our scenario works a bit strange at the moment and it seems Dart compiler does too much work not taking advantage of caching.

I will also continue investigating ways to solve this issue and will keep this PR open if you don't mind (or we can convert it to issue).

[incendial]

Also we can investigate #313 and it can solve the problem. Either way it requires a deeper research.

[roman-petrov]

sure, #313 may partially solve the problem (after the underlying Dart SDK issue is fixed) but we still have it when running in dart code metrics CLI

[incendial]

but we still have it when running in dart code metrics CLI

Agreed. I'll take a closer look after 4.0 is released.

[roman-petrov]

@incendial - at the moment do not have time and ideas that would quickly fix this performance problem. Maybe it will be better to close this PR for now?

[incendial]

@roman-petrov I think we can try some ideas, but I don't think that it's a quick fix situation. Let's have the PR open for now, we should probably increase the priority of this problem.

[roman-petrov]

ok, let's leave it here for now. I also may put some effort into this in the future since linting speed is important for large / relatively large codebases.

Although I have a quick idea. You have a GitHub acition but it is not integrated into the dart code metrics repository itself. Maybe, it would be better to add this action first to "see" CLI performance.

[incendial]

@roman-petrov is it a blocker for you to start using the package? Think that might affect how we prioritize this.

[roman-petrov]

@incendial, yes, the performance issue is almost blocking us from starting. But at the moment it is not critical for us: we have a huge amount of other things to do while migrating our UI development technology stack to Flutter.

[incendial]

@roman-petrov then I need to know what is a "good enough" performance for you and are you ready to not being able to use some analysis lints if it will be the only way to fix the problem?

[incendial]

I mean, some rules just won't work without full dependencies resolution (especially, flutter rules).

[roman-petrov]

@roman-petrov then I need to know what is a "good enough" performance for you and are you ready to not being able to use some analysis lints if it will be the only way to fix the problem?

Actually, I don't have an exact answer to this question. I would say something like: the performance will be good enough if the time dart_code_metrics consumes while analyzing the project will have the same order that dart analyze consumes with almost all lint rules enabled.

For example, if dart analyze takes 2 seconds, then it would be ok if dart_code_metrics takes something like one, two, or even five seconds. But 20 seconds, two minutes, five minutes might be not acceptable in terms of effective workflow.

[roman-petrov]

Actually, as I understand, the performance issue we discuss is somehow related to getResolvedUnit method in analyzer package. So we don't have to optimize rules themselves, we should think about optimizing program flow somehow.

Just another "draft" idea: dart analyze even when running from CLI uses analysis server. Maybe the same can be done in dart_code_metrics CLI... I am not familiar enough with the dart analyzer, so I am not sure if this approach can help.

[incendial]

For example, if dart analyze takes 2 seconds, then it would be ok if dart_code_metrics takes something like one, two, or even five seconds. But 20 seconds, two minutes, five minutes might be not acceptable in terms of effective workflow.

Got it, sounds pretty reasonable.

Just another "draft" idea: dart analyze even when running from CLI uses analysis server. Maybe the same can be done in dart_code_metrics CLI... I am not familiar enough with the dart analyzer, so I am not sure if this approach can help.

Yeah, thats one way to look at it, but we definitely need a more deep research.

[incendial]

I think we finally find out what was the problem, closing due to: #428.