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

analyzer takes a long time to process deletion of empty file on linux #55560

Open
Hixie opened this issue Apr 24, 2024 · 8 comments
Open

analyzer takes a long time to process deletion of empty file on linux #55560

Hixie opened this issue Apr 24, 2024 · 8 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@Hixie
Copy link
Contributor

Hixie commented Apr 24, 2024

STEPS TO REPRODUCE

  1. Run flutter analyze --watch in a big package, e.g. the flutter framework (https://github.com/flutter/flutter/tree/master/packages/flutter)
  2. From a separate terminal, create an empty file in that package, e.g. touch foo.dart
  3. Delete that file, e.g. rm foo.dart

EXPECTED RESULTS

Step 1 should take a while. Steps 2 and 3 should each trigger a very fast reanalysis.

ACTUAL RESULTS

Step 1 takes a while. Step 2 triggers a fast analysis. Step 3 triggers a multi-second analysis.

CONTEXT

This impacts users of IDEs that create temporary files with .dart extensions. For example, Emacs creates advisory lock files with names like .#animated_size_test.dart when a file is edited and not saved. (Actually in the case of Emacs it's not even a file, it's a symlink to a non-existent file, but the effect on the analyzer is the same.) When a file is saved, reanalysis takes multiple seconds every time even though nothing has changed, because the editor deletes this temporary file with a .dart extension.

@Hixie
Copy link
Contributor Author

Hixie commented Apr 24, 2024

cc @DanTup

@bwilkerson
Copy link
Member

@scheglov

@DanTup
Copy link
Collaborator

DanTup commented Apr 25, 2024

@scheglov I had a quick look at this, and with some extra logging I can see that the deletion seems to trigger analyzing a lot of files:

add new file

1714040979301:Watch:<unknown>:C::\Dev\Google\Flutter\Flutter main\packages\flutter\foo.dart:add
1714040979303:Noti:{"event"::"server.status","params"::{"analysis"::{"isAnalyzing"::true}}}
1714040979304:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\foo.dart
1714040979305:Noti:{"event"::"analysis.errors","params"::{"file"::"C::\\Dev\\Google\\Flutter\\Flutter main\\packages\\flutter\\foo.dart","errors"::[]}}
1714040979307:Noti:{"event"::"server.status","params"::{"analysis"::{"isAnalyzing"::false}}}

delete file

1714040984594:Watch:<unknown>:C::\Dev\Google\Flutter\Flutter main\packages\flutter\foo.dart:remove
1714040984596:Noti:{"event"::"analysis.flushResults","params"::{"files"::["C::\\Dev\\Google\\Flutter\\Flutter main\\packages\\flutter\\foo.dart"]}}
1714040984596:Noti:{"event"::"server.status","params"::{"analysis"::{"isAnalyzing"::true}}}
1714040984601:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_profile\basic_test.dart
1714040984674:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_release\diagnostics_test.dart
1714040984674:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_release\foundation\memory_allocations_test.dart
1714040984674:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_release\precondition_test.dart
1714040984674:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_release\rendering\memory_allocations_test.dart
1714040984674:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\test_release\widgets\memory_allocations_test.dart
1714040984675:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\animation.dart
1714040984696:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\cupertino.dart
1714040984696:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\foundation.dart
1714040984696:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\gestures.dart
1714040984696:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\material.dart
1714040984704:Info:Analyzing general file C::\Dev\Google\Flutter\Flutter main\packages\flutter\lib\painting.dart

Looking in the code, I came across this line that adds all of addedFiles to _pendingFiles when a file is removed (and a TODO that may be related):

// TODO(paulberry): removing the path from [fsState] and re-analyzing all
// files seems extreme.
_fsState.removeFile(path);
_pendingFiles.addAll(addedFiles);

I guess the intention is to ensure files that depend on the deleted file are re-analyzed. I don't know how much of that work is short-cut or whether it's possible to be more selective (in this case, 0 files depend on the deleted one). On my machine, the time spent re-analyzing for the Flutter repo is about 4s on the deletion.

@dart-github-bot
Copy link
Collaborator

Item Details
Summary Analyzer takes a long time to process deletion of empty file on Linux.
Triage to area-analyzer

(what's this?)

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) labels Apr 25, 2024
@pq pq added the P2 A bug or feature request we're likely to work on label Apr 25, 2024
@scheglov
Copy link
Contributor

The analysis is correct, deleting a file is currently very expensive.
Improving here will require a lot of thinking and testing, for which we don't have time right now.

@DanTup
Copy link
Collaborator

DanTup commented Apr 25, 2024

@Hixie as a workaround for you, you might be able to change your editor to not write these lock files to the project - see https://www.emacswiki.org/emacs/LockFiles

It does say "but it might not be wise to do this" without an explanation though 🥲

@Hixie
Copy link
Contributor Author

Hixie commented Apr 25, 2024

Yeah, I've changed my configuration already. :-)

@Hixie
Copy link
Contributor Author

Hixie commented Apr 25, 2024

Does the analyzer have a graph of which files affect each other?

@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants