Skip to content
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

Using 'source.fixAll' in codeActionsOnSave makes servers very slow (but running the "Fix All" command directly is fast) #3952

Closed
nickmeinhold opened this issue May 5, 2022 · 9 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is performance
Milestone

Comments

@nickmeinhold
Copy link

nickmeinhold commented May 5, 2022

I'm generating a large Dart file (~3.5k lines) and the analyzer seems fine but when I make a change and save I see:

pic

and the save never completes.

If I turn off 'fixAll' and 'organizeImports' under 'codeActionsOnSave' the save completes without issue.

Logs:
Dart-Code-Log-2022-04-04 12-00-19.txt

Thanks!

@DanTup DanTup added this to the v3.42.0 milestone May 5, 2022
@DanTup DanTup added in editor Relates to code editing or language features is performance in lsp/analysis server Something to be fixed in the Dart analysis server labels May 5, 2022
@DanTup
Copy link
Member

DanTup commented May 5, 2022

@nickmeinhold does this happen if you just run the fix command manually? Are you able to repro in a file you can share? I tried in a file with 33k lines of code and couldn't reproduce (it did take a few tens of seconds to format/organize/fix, though it's large file with a ton of wrapping that I know takes the formatter a little while to wrap).

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label May 5, 2022
@nickmeinhold
Copy link
Author

nickmeinhold commented May 5, 2022

Yes dart fix --apply takes ~7 seconds but works fine.

This is the badly behaving file:
b2_adapters_web.dart.zip

And this is the file in situ in case that's useful
https://github.com/enspyrco/monorepo/blob/main/packages/flutter-plugins/flutter_box2d/flutter_box2d_web/lib/b2_adapters_web.dart

@DanTup
Copy link
Member

DanTup commented May 5, 2022

Perfect, thanks! I seem to be able to trigger this (and it eats 100% CPU when it occurs). I'll see what I can find.

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label May 5, 2022
@DanTup
Copy link
Member

DanTup commented May 5, 2022

@nickmeinhold when you see this, how many errors do you have in that file? I can repro this taking a long time having taken that file in isolation, and most of it is in producing fixes for the non_constant_identifier_names errors. I'm looking into this, but I'm wondering whether it's the same thing you're seeing, or I might be looking at something different to you.

@nickmeinhold
Copy link
Author

The file changes quite significantly because I'm generating it from a python script (which I'm actively working on and making changes to) and often making a small change to the python results in large changes to the dart file. Having said that a lot of the required fixes like non_constant_identifier_names tend not to change.

The issue of not being able to save has been very consistent despite large changes to the Dart file though.

My workflow is usually: make changes to the python script, generate a new Dart file, view the Dart file and maybe try a small change in the Dart to see what the effect will - so I've also wondered if generating the file while the editor has it open might cause problems?

I'm not sure if I adequately answered your original question, please let me know if not so I can try again :-)

@nickmeinhold
Copy link
Author

I updated the python script to add the following to the top of the generated Dart file

// ignore_for_file: non_constant_identifier_names
// ignore_for_file: annotate_overrides

which reduced the number of warnings from ~1000 to 30 and saving now seems to go smoothly - I guess the fix also respects the ignore_for_file comment? Anyway that has solved the problem for me and happy to close the issue or would you like it left open?

Thanks very much!

@DanTup
Copy link
Member

DanTup commented May 7, 2022

Great, thanks for confirming!

I'll keep this open, as it's taking a lot longer in VS Code than dart fix is, so something still seems wrong. From a quick glance of the logs, it seems like VS Code might be asking the language server for every quickfix for the entire file just before it does this (not just those that can be applied en-mass), so I think the time is being spent computing fixes that would never be run by the "fix all" command.

If that's the case, the fix may be an optimisation in VS Code, but I'll dig into this more concretely next week. Thanks!

@DanTup DanTup changed the title Can't save large file with 'fixAll' and 'organizeImports' on Using 'source.fixAll' in codeActionsOnSave makes servers very slow (but running the "Fix All" command directly is fast) May 9, 2022
@DanTup
Copy link
Member

DanTup commented May 9, 2022

What VS Code was doing was fine, it was specifically asking us only for the source.fixAll fixes. However, for some types of fixes, the server was always computing them and then filtering them out afterwards when in many cases (like this one) it could have skipped the work entirely.

Working on a fix, will close this when it lands.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 9, 2022
… with no range

Fixes Dart-Code/Dart-Code#3952.

Change-Id: Ie523dc628ab72705259810baccafa949af96280e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244101
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Member

DanTup commented May 17, 2022

The underlying cause here should be fixed by dart-lang/sdk@3c0289d (which will ship in a future SDK release).

@DanTup DanTup closed this as completed May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is performance
Projects
None yet
Development

No branches or pull requests

2 participants