-
Notifications
You must be signed in to change notification settings - Fork 30.1k
Description
Steps to reproduce
Pigeon's tool/generate.dart tool applies many, many incremental writes. In order to touch a few dozen files in example/ and platform_tests/, I logged ~166k writes. This stems from the using the Indent class to write to the IOSink many times. This is a common pattern when the IOSink is a StringBuffer, but has performance consequences when it is the sink returned by dart:io's File.openWrite.
First, thanks to @davidmorgan for the steps to count the file writes; thanks to @keertip for narrowing this issue to Linux; thanks to @davidmorgan and @mraleph for insights regarding Linux, Mac, and Windows file-watching differences; thanks to @tarrinneal for the original reproduction steps.
We think this only reproduces on Linux, whose file watching utility (inotify) does not batch events. The utilities on Mac and Windows somehow batch things. (Or possibly the interfaces in the Dart watcher package batch them, but this problem was seen to peg VS Code to 200% CPU, only on Linux, so I don't think the batching is isolated to the Dart watcher package.)
The user-facing issues that this creates are (1) pegging VS Code CPU usage and (2) a long delay in Dart analysis server actions which also results from sifting through hundreds of thousands of watch events.
Repro steps
- Install
inotify-tools. I think this is only available on linux platforms. I usedapt install inotify-tools. This putsinotifywaiton my PATH. - Checkout this flutter/packages repo. Set up development for
packages/pigeon(I just runflutter pub getinpackages/pigeon, and in each pacakge inpackages/pigeon/platform_tests). - In one terminal,
cd packages/pigeon, andinotifywait -m -r . | tee /tmp/log. Leave this running. - In a separate terminal,
cd packages/pigeon, anddart tool/generate.dart. - The first terminal should get spammed by file events. After the generate command in the second command finishes,
CTRL+Cto killinotifywait. Thenwc -l /tmp/logshows > 166k events.
I don't have my path or... dart install setup or whatever, so dart run script/tool/bin/flutter_plugin_tools.dart format doesn't work. This is fine by me as I automatically exclude any file operations from the formatting tools.
Possible solution
I have some ideas for a solution that should be very simple to implement: Right now all disk writes to the generated files are directly through the Indent class which operates on an IOSink.
- We can change the callers of
Indentto instead pass a StringBuffer, and then the callers must write the contents of the StringBuffer to theFile.openWritehandle. - OR, we can just change Indent's internal implementation to make all of its writes to a StringBuffer. Then at some signal from outside Indent (like a close signal or something), it writes the contents of the StringBuffer to the given
IOSink.
@davidmorgan identified a few other places that could be improved to better handle many file-watch events, but it would still be super beneficial to change pigeon's behavior here.
- Dedupe events better in analyzer: _applyPendingFileChanges is O(known libraries) and does not dedupe events dart-lang/sdk#62760
- Add buffering support to
File.openWrite: IOSink and many small .adds is slow dart-lang/sdk#32874
Expected results
Few writes
Actual results
Many writes
Code sample
No thank you.
Screenshots or Video
No response
Logs
No response
Flutter Doctor output
$ flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel master, 3.42.0-1.0.pre-290, on Debian GNU/Linux rodete 6.16.12-1rodete2-amd64, locale en_US.UTF-8)
[✗] Android toolchain - develop for Android devices
✗ Unable to locate Android SDK.
Install Android Studio from: https://developer.android.com/studio/index.html
On first launch it will assist you in installing the Android SDK components.
(or visit https://flutter.dev/to/linux-android-setup for detailed instructions).
If the Android SDK has been installed to a custom location, please use
`flutter config --android-sdk` to update to that location.
[✓] Chrome - develop for the web
[✗] Linux toolchain - develop for Linux desktop
✗ CMake is required for Linux development.
It is likely available from your distribution (e.g.: apt install cmake), or can be downloaded from https://cmake.org/download/
✗ ninja is required for Linux development.
It is likely available from your distribution (e.g.: apt install ninja-build), or can be downloaded from https://github.com/ninja-build/ninja/releases
✗ GTK 3.0 development libraries are required for Linux development.
They are likely available from your distribution (e.g.: apt install libgtk-3-dev)
[✓] Connected device (2 available)
[✓] Network resources
! Doctor found issues in 2 categories.