Profile gn gen and no-op engine builds.

[chinmaygarde]

This based on an observation that no-op engine builds during development were getting steadily slower over time.

Usually, even for no-op builds, Engine developers run gn gen (invoked by ./flutter/tools/gn) followed by the ninja invocation to generate build artifacts. Just running ninja is sufficient to pick up changes to GN rules themselves but this does not pick up changes to GN args. So, most just run the two invocations in order. This workflow has been getting alarmingly slow over time.

On my eng Macbook Pro, gn gen takes ~4s today. It used to take less than 300ms earlier (say 3 years ago).

$ time ./flutter/tools/gn --runtime-mode debug
Generating GN files in: out/host_debug
Generating Xcode projects took 241ms
Done. Made 739 targets from 228 files in 1608ms
./flutter/tools/gn --runtime-mode debug  3.98s user 3.40s system 263% cpu 2.799 total

Just running ninja on the other hand seems to take 300ms which seems fine to me.

$ time ninja -C out/host_debug_unopt -j999
ninja: Entering directory `out/host_debug_unopt'
ninja: no work to do.
ninja -C out/host_debug_unopt -j999  0.29s user 0.12s system 98% cpu 0.425 total

Taking >4s for a no-op builds breaks flow. I used to be able to build on save and I can't do that anymore. Also, given the trend, this is probably going to get worse without some effort to arrest the increase. I can't tell how much of the increase is from additional targets, additional use of scripts in GN, or, just the effects of running GN on my laptop in the WFH environment vs. using the beasts in the office.

GN and Ninja have built in tools to profile the various steps and we should track what the most expensive steps are. Typically, the guidance has been to avoid Python or Dart scripts referenced in GN rules as they are typically the bottleneck. To avoid having these scripts, newer versions of GN have built-in functions that we could probably use. I am positive there are tons of low hanging fruit that will significantly improve productivity of engine developers. Separately, we don't have (or adhere to) documented best practices for authoring GN rules. Perhaps now is the time to set that up. Also, perhaps there is a way to benchmark how long no-op builds take so we can figure out trends.

[chinmaygarde]

Ok, that was easier than I expected. As anticipated, it was script execution. A couple of the scripts, we can get rid of and use GN built-ins. There are some that can be parallelized. Generating Xcode projects can also probably be opt-in. Adding --tracelog=<file> in gn gen generates the following trace for just that one step.

[chinmaygarde]

The ios_sdk.py and mac_sdk.gni bottlenecks can be tackled by being smarter about when and how many times we generate symlinks for GOMA (cc @dnfield).

The bottlenecks then become script executions in Dart. Specifically, sdk_args.gni is in an import so it get executed multiple times and is slow. Perhaps that can be put in an arg. list_dart_files.py also seems to be slow and performing tasks redundantly. Is it possible to list the sources directly in GN rules like we do for all other targets? (cc @a-siva @zanderso).

Then there a are a couple scripts in Dart as well as engine that attempt to read Git revisions and such for versioning. This can also be done once upfront and stashed away in a GN variable.

Once these are tackled, I believe everything should complete in well under a second.

[chinmaygarde]

cc @aam @rmacnak-google: Is there a way we can reduce the usage of script files in GN?

[rmacnak-google]

I expect all the list_dart_files.py can be removed; I believe it predates the ability of our tools to generate depfiles.

[chinmaygarde]

xref https://dart-review.googlesource.com/c/sdk/+/196981. Thanks! I'll stash the iOS and Mac SDK details in a GN variable. I already patched compile_commands.json generation so its now not a separate step (saves 1 second). With the three fixes, I think we should be well under two seconds. I'll see if we can get rid of engine versioning from Git SHAs. We never exposed that to users so I don't think its absolutely necessary (we do use it to version caches).

[dnfield]

I'll see if we can get rid of engine versioning from Git SHAs

We do have a test in the framework that verifies flutter_tester --help outputs the same hash that we asked to download from GCS:

flutter/dev/bots/test.dart

Lines 137 to 146 in b8833af

	final String expectedVersion = File(engineVersionFile).readAsStringSync().trim();
	final CommandResult result = await runCommand(flutterTester, <String>['--help'], outputMode: OutputMode.capture);
	final String actualVersion = result.flattenedStderr.split('\n').firstWhere((final String line) {
	return line.startsWith('Flutter Engine Version:');
	});
	if (!actualVersion.contains(expectedVersion)) {
	print('${red}Expected "Flutter Engine Version: $expectedVersion", '
	'but found "$actualVersion".');
	exit(1);
	}

This helps protect against issues where a bug in a builder causes it to upload the wrong git hash to a particular bucket, e.g. it thinks it's building head but it's not

[jason-simmons]

dart-lang/sdk@8d436a2 is causing unnecessary operations to run in no-op engine builds.

If I run:

• flutter/tools/gn --unoptimized
• ninja -C out/host_debug_unopt
• ninja -C out/host_debug_unopt again

the second run of ninja should do nothing. With the above change, it's rerunning the list_dart_files_as_depfile.py actions and the Dart snapshotting actions that depend on those depfiles.

Can you revert dart-lang/sdk@8d436a2?

Regarding the broader issue highlighted in this bug: in my workflow I typically only need to run gn after syncing to a new engine commit or selecting a new target configuration. Rebuilding the engine after changing source code should only require running ninja, which is generally efficient.

@chinmaygarde what scenarios require frequently rerunning gn?

[chinmaygarde]

@jason-simmons Can you run ninja -d explain -C out/host_debug_unopt on the second run to see why ninja is doing work. AFAIK, such issues occur because of subtle difference in depfile listings. For instance, absolute vs relative paths.

[jason-simmons]

Based on the explain output it looks like ninja expects the depfile to contain a relative path to the depfile itself.

@rmacnak-google I can get this to work as expected in the Flutter engine build by changing dart-lang/sdk@8d436a2#diff-40dd701de08faf64c714b08263634be72ff06ced2cbb199e67c5ec4f516c2d9eR32
to use os.path.relpath(depfile)

[zanderso]

@jason-simmons I believe @rmacnak-google is OoO today, so if you have something working locally, sending out a CL for it is probably the fastest thing.

[jason-simmons]

The depfile script was reverted in the Dart SDK: dart-lang/sdk@5c0ff97