Skip to content

[pigeon] Generating code applies too many incremental writes. #182958

@srawlins

Description

@srawlins

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

  1. Install inotify-tools. I think this is only available on linux platforms. I used apt install inotify-tools. This puts inotifywait on my PATH.
  2. Checkout this flutter/packages repo. Set up development for packages/pigeon (I just run flutter pub get in packages/pigeon, and in each pacakge in packages/pigeon/platform_tests).
  3. In one terminal, cd packages/pigeon, and inotifywait -m -r . | tee /tmp/log. Leave this running.
  4. In a separate terminal, cd packages/pigeon, and dart tool/generate.dart.
  5. The first terminal should get spammed by file events. After the generate command in the second command finishes, CTRL+C to kill inotifywait. Then wc -l /tmp/log shows > 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.

  1. We can change the callers of Indent to instead pass a StringBuffer, and then the callers must write the contents of the StringBuffer to the File.openWrite handle.
  2. 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.

  1. Dedupe events better in analyzer: _applyPendingFileChanges is O(known libraries) and does not dedupe events dart-lang/sdk#62760
  2. 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.

Metadata

Metadata

Assignees

Labels

p: pigeonrelated to pigeon messaging codegen toolpackageflutter/packages repository. See also p: labels.team-ecosystemOwned by Ecosystem team

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions