Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short-circuit crawling all targets in et {build|test|query} #52832

Merged
merged 11 commits into from
May 15, 2024

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]

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
26 checks passed
@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
…148430)

flutter/engine@41b86b5...cd15098

2024-05-15 skia-flutter-autoroll@skia.org Roll Skia from b9a304eb05d2 to 0f737206b709 (3 revisions) (flutter/engine#52858)
2024-05-15 matanlurey@users.noreply.github.com Short-circuit crawling all targets in `et {build|test|query}` (flutter/engine#52832)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants