Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@matanlurey
Copy link
Contributor

Closes flutter/flutter#147666.

This is a large change, I'd be happy to either review synchronously or change commands one at a time, but I think this is overall the right approach. I also didn't see any reason to reuse the BuildRunner code for these commands, the flow is basically:

graph LR
    A[et targetsOrPatterns] --> B[gn desc --format=json]
    B --> C[existing code that runs ninja/workers]
Loading

Quick summary of changes:

  • Introduced a Label and TargetPattern type to avoid awkward string parsing/manipulation all over
  • Replaced gn_utils (i.e. targetsFromCommandLine and friends) with an invocation of gn desc
  • Some tests were brittle in terms of expected output, I mostly left them alone and wrote TODOs where applicable

Here is the fun part, results (and some manual integration tests):

# build, previously was 10-15s, is now ~3s
flutter % time et build -c host_debug_unopt_arm64 //flutter/impeller:impeller_unittests
[2024-05-14T19:05:55.163][macos/host_debug_unopt_arm64: GN]: OK
[2024-05-14T19:05:56.119][macos/host_debug_unopt_arm64: RBE startup]: Proxy started successfully.
[macos/host_debug_unopt_arm64: ninja] 0.0% (0/1) Regenerating ninja files[2024-05-14T19:05:57.309][macos/host_debug_unopt_arm64: ninja]: OK
[2024-05-14T19:05:57.573][macos/host_debug_unopt_arm64: RBE shutdown]: Actions completed: 0
$ENGINE/flutter/bin/et build -c host_debug_unopt_arm64   3.20s user 0.87s system 107% cpu 3.776 total

# test, also benefits from speedup, but I didn't time it
flutter % et test -c host_debug_unopt_arm64 //flutter/fml:fml_unittests
[2024-05-14T19:07:01.843][macos/host_debug_unopt_arm64: GN]: OK
[2024-05-14T19:07:02.707][macos/host_debug_unopt_arm64: RBE startup]: Proxy started successfully.
[2024-05-14T19:07:07.400][macos/host_debug_unopt_arm64: ninja]: 100.0% (3/3) LINK ./fml_unittests
[2024-05-14T19:07:07.404][macos/host_debug_unopt_arm64: ninja]: OK
[2024-05-14T19:07:07.748][macos/host_debug_unopt_arm64: RBE shutdown]: Actions completed: 1 (1 racing local)
OKAY:         7s.95ms //flutter/fml:fml_unittests

# query, also benefits from speedup
flutter % time et query targets -c host_debug_unopt_arm64
//flutter/display_list:display_list_benchmarks
//flutter/display_list:display_list_builder_benchmarks
# ... many targets omitted ...
$ENGINE/flutter/bin/et query targets -c host_debug_unopt_arm64  1.27s user 0.18s system 147% cpu 0.978 total

In other words, ~5x improvement on et build and et query is much faster as well.

@matanlurey matanlurey merged commit ffa9513 into flutter:main May 15, 2024
@matanlurey matanlurey deleted the gn-label branch May 15, 2024 19:57
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 15, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 15, 2024
auto-submit bot pushed a commit that referenced this pull request May 16, 2024
…y}` (#52866)

Closes flutter/flutter#148442.

This restores functionality that existed prior in #52832:

- Splits `runGn` to `ensureBuildDir`, which is in practice what it does.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[et] no-op builds when a target is specified take ~10 to 15 seconds instead of < 0.1 seconds with Ninja directly.

2 participants