Skip to content

Analysis Server slows down when Save performed #50960

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

Closed
maRci002 opened this issue Jan 9, 2023 · 12 comments
Closed

Analysis Server slows down when Save performed #50960

maRci002 opened this issue Jan 9, 2023 · 12 comments
Assignees
Labels
analyzer-stability legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@maRci002
Copy link

maRci002 commented Jan 9, 2023

Steps to reproduce

In Android Studio when File | Settings | Languages & Frameworks | Flutter | Format code on save or File | Settings | Tools | Actions on Save | Reformat code is enabled OR WITHOUT THEM and Save action is performed the Analyzer Server is not avaible for the saved file for a long time ~10 sec.

In Visual Studio Code everything working fine.

Example code
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return const Placeholder();
  }
}

Here is a little video both Rename F2 and Context Actions Alt + Enter are working and whenever a Save Action is performed Ctrl + S the Analyzer won't suggest anything for more then 10 seconds and Rename is still not avaliable.

android_studio.mp4

As you can see Bloc's plugin suggestions working fine however I cannot get any suggetsion for a long time from the Analyzer Server.

Sometimes when Format code on save is enabled and I delete some method so some Class definition jumps into it's place then Rename might want to rename some method or it's constructors key paramter <= I think Analyzer Server doesn't get updated file. Edit: I made a seperate issue for this problem: #50968

I can reproduce on both stable and beta channels.

dart --version

Dart SDK version: 2.19.0-444.3.beta (beta) (Wed Jan 4 15:18:31 2023 +0000) on "windows_x64"

flutter --version

Flutter 3.7.0-1.3.pre • channel beta • https://github.com/flutter/flutter.git
Framework • revision 9b4416aaa7 (5 days ago) • 2023-01-04 17:29:34 -0600
Engine • revision 7b8906637f
Tools • Dart 2.19.0 (build 2.19.0-444.3.beta) • DevTools 2.20.0

Android Studio Dolphin | 2021.3.1 Patch 1
Build #AI-213.7172.25.2113.9123335, built on September 30, 2022
Runtime version: 11.0.13+0-b1751.21-8125866 amd64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
Windows 10 10.0
GC: G1 Young Generation, G1 Old Generation
Memory: 2048M
Cores: 16
Registry:
    external.system.auto.import.disabled=true
    ide.text.editor.with.preview.show.floating.toolbar=false

Non-Bundled Plugins:
    idea.plugin.protoeditor (213.6461.28)
    com.markskelton.one-dark-theme (5.7.2)
    com.bloc.intellij_generator_plugin (3.4.0)
    Dart (213.7433)
    andrasferenczi.dart-data-plugin (0.2.0)
    io.flutter (71.2.3)
@mraleph mraleph added the legacy-area-analyzer Use area-devexp instead. label Jan 10, 2023
@mraleph
Copy link
Member

mraleph commented Jan 10, 2023

/cc @jensjoha for visibility

@jensjoha
Copy link
Contributor

Nice.

6de7ea7 is the reason --- a timer delays the analysis for 10 seconds and in the meantime it won't answer.

In particular, when editing the file analysis.updateContent event of type add is sent together with the new content of the file and everything's fine. Then upon saving the file a analysis.updateContent event of type remove is sent which goes here:

} else if (change is RemoveContentOverlay) {
_pendingFilesToRemoveOverlay.remove(file)?.cancel();
_pendingFilesToRemoveOverlay[file] = Timer(
pendingFilesRemoveOverlayDelay,
() {
_pendingFilesToRemoveOverlay.remove(file);
resourceProvider.removeOverlay(file);
_changeFileInDrivers(file);
},
);
return;
} else {

I'll assign this to @scheglov who authored the introduction of this. It seems to have good intentions (from one of the comments in the commit "This helps for analysis server running remotely, with slow remote file systems") but miss the mark in letting everyone have this delay.

@mraleph
Copy link
Member

mraleph commented Jan 11, 2023

Thanks @jensjoha!

/cc @srawlins for prioritisation. seems like potentially high negative UX impact?

@srawlins
Copy link
Member

Yes I imagine this could be the root cause of much of our performance complaints. 6de7ea7 was also the decided culprit in #49107.

@srawlins srawlins added analyzer-stability type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 11, 2023
@jensjoha
Copy link
Contributor

From #50968 it's probably furthermore the mismatching line-endings between saved and not-saved when the IDE fires requests that make it respond as it does. This means that it's probably only an issue on Windows. If the previous overlay and the new file data has been identical (which it probably is on Mac and Linux) it probably wouldn't be that big of an issue.

@jensjoha
Copy link
Contributor

Oh, depending on how "format on save" is implemented that could possibly introduce a mismatch on non-Windows too. I haven't looked into that though.

@bwilkerson
Copy link
Member

@jwren

@scheglov
Copy link
Contributor

copybara-service bot pushed a commit that referenced this issue Jan 11, 2023
This reverts 6de7ea7

This hurts our external users, and I got an impression that the
scenario for which we designed it initially is not the way our
internal developers works now.

Bug: #50960
Change-Id: I9b1ecf4860146d67963c5c99ecefc581e1526a89
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278883
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
@scheglov scheglov added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Jan 11, 2023
@AlexV525
Copy link
Contributor

Can we CP the fix to 2.19?

@mraleph
Copy link
Member

mraleph commented Jan 13, 2023

Yes, this most certainly needs to be cherry picked once it bakes a bit.

@srawlins could you make a CP request?

@srawlins
Copy link
Member

Can do!

copybara-service bot pushed a commit that referenced this issue Jan 17, 2023
Issue 50960. Revert delaying removing file overlays.

This reverts 6de7ea7

This hurts our external users, and I got an impression that the
scenario for which we designed it initially is not the way our
internal developers works now.

Bug: #50960
Change-Id: I9b1ecf4860146d67963c5c99ecefc581e1526a89
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278883
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278993
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
@maRci002
Copy link
Author

Thank you Flutter team for the fast fix, I can confirm the prebuilt Dart 2.19.0 which ships via Flutter 3.7.0 has the above changes and works fine.

Before I updated Flutter, I tried one more thing, so that the new lines of the file have unix line separator instead of windows line separator, and everything started to work so 6de7ea7 might be readded however it should work without this workaround. I also tested git autocrlf true but it doesn't change line breaks only after commit.

unix line endings

@jensjoha was right about line endings:

From #50968 it's probably furthermore the mismatching line-endings between saved and not-saved when the IDE fires requests that make it respond as it does. This means that it's probably only an issue on Windows. If the previous overlay and the new file data has been identical (which it probably is on Mac and Linux) it probably wouldn't be that big of an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-stability legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants