Regression in compilation time due to native asset bundling

[zanderso]

SkiaPerf is reporting a 0.5 - 1s regression in debug mode compilation time due to #130494.

https://flutter-flutter-perf.skia.org/e/?begin=1694274733&end=1694422824&keys=Xb9306954671fd63586c21f618c09ccd6&num_commits=50&request_type=1&xbaroffset=36775

I'm a bit concerned because the program being compiled for the benchmark doesn't use the new feature added by the PR (?)

cc @dcharkes @christopherfujino @stuartmorgan

[dcharkes]

My first hunch is that it might be the file access for all dependencies to check if there's a build.dart in the package root.

Let me take a look.

[dcharkes]

I observe a slowdown locally on flutter build ios --release --tree-shake-icons --split-debug-info=infos/ in the hello world example after a flutter clean && flutter pub get of ~400ms. The difference between the fastest runs (out of 5) before and after the commit is 400 ms. (The runs vary all the way from 16 to 21 seconds though.)

One thing I see in the logs is the new NativeAssets Target taking 200 milliseconds:

           [   +2 ms] native_assets: Starting due to {}
           [   +2 ms] Skipping target: gen_localizations
           [   +1 ms] gen_dart_plugin_registrant: Starting due to {InvalidatedReasonKind.inputChanged: The following inputs have updated contents: /Users/dacoharkes/flt/engine/flutter/examples/hello_world/.dart_tool/package_config_subset}
           [  +33 ms] gen_dart_plugin_registrant: Complete
           [ +107 ms] release_unpack_ios: Complete
           [  +60 ms] Writing native_assets.yaml.
           [   +7 ms] Writing /Users/dacoharkes/flt/engine/flutter/examples/hello_world/.dart_tool/flutter_build/be2692bbfbc0b9a27fcd2422d52354c6/native_assets.yaml done.
           [        ] native_assets: Complete

In these 200 milliseconds, it spends 150 ms loading/parsing the PackagesConfig. We should already have a packages config somewhere in the Flutter tools, so we should be using that instead of loading it from disk and reparsing it.

(The reason we do work at all with the new feature being disabled, is that we give an error message if any dependency has native assets, but the feature is not enabled.)

I'll (1) make a PR to not reparse the package config, and (2) see if I can find anything else which has been slowed down.

[dcharkes]

The time spent in the native_assets target is not actually spent doing native assets stuff. It's in awaits while other targets do sync I/O. Targets doing sync I/O identified so far release_unpack_ios and gen_dart_plugin_registrant.

However, this does increase the latency before the kernel_snapshot target is started, leading to a slower build overall.

I have made #134523 which converts a some of sync file IO and process runs to to the non-sync ones. This changes the log to:

           [   +4 ms] native_assets: Starting due to {}
           [        ] Skipping target: gen_localizations
           [   +1 ms] gen_dart_plugin_registrant: Starting due to {InvalidatedReasonKind.inputChanged: The following inputs have updated contents: /Users/dacoharkes/flt/engine/flutter/examples/hello_world/.dart_tool/package_config_subset}
           [  +31 ms] Writing native_assets.yaml.
           [   +8 ms] Writing /Users/dacoharkes/flt/engine/flutter/examples/hello_world/.dart_tool/flutter_build/f9451a65a465bfab70d004e21d6cc1d6/native_assets.yaml done.
           [   +1 ms] native_assets: Complete

The question now is, should I/O be sync or async in the backend?
Since the backend processes targets in parallel, I believe using sync I/O in them is bad.
Who can I ask for some insight on the architecture in the backends? @zanderso @stuartmorgan @christopherfujino

[zanderso]

Converting to async I/O sounds like the right choice as long as the Dart code isn't holding onto any instances of RandomAccessFile.

[dcharkes]

When the native assets feature is enabled, computing the order in which native assets should be built with dart pub deps --json takes 250 milliseconds.

$ pwd
/Users/dacoharkes/flt/engine/flutter/examples/hello_world
$ time dart pub deps --json
# ...
real	0m0.248s
user	0m0.241s
sys	0m0.069s

I can tweak the build process slightly, so that if we have <= 1 packages with native assets, we don't have to compute the topological ordering.

cc @jonasfj once this feature is used and we have >= 2 packages with native assets, we might not want to incur a 250 milliseconds latency.

[dcharkes]

8ebb8d4 brought the build times back to where they were.

https://flutter-flutter-perf.skia.org/e/?keys=Xb9306954671fd63586c21f618c09ccd6&selected=commit%3D36882%26name%3D%252Carch%253Darm%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Ddebug_full_compile_millis%252Ctest%253Dhello_world_ios__compile%252C&xbaroffset=36775

Thanks for the ping @zanderso!

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.