Removes reported errors in VSCode when using `clangd`
* Parsing a 21MB json string is not performant
* RegExp replacing all occurrences in said 21MB string is (~61ms)
* Fixes#147767
Revert label failed due to conflicts
`FlutterFragmentTest.java` was the only file that had merge issues, everything else is the output of `git revert 802e5d2cd3c9e73f336e3fe43487b64a5fdf98d8`
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
We don't support 32-bit arm iOS builds anymore. But adding the --ios-cpu=arm64 appends the "_arm" to the out subdirectory. This just causes confusion since the ios_debug_unopt is still arm64.
Just remove the flag that does nothing.
Upgrades every `androidx` dependency in the `src/third_party/android_embedding_dependencies` bundle to the latest version, except the `lifecycle` group*. Tested running a couple of apps as well because when updating these dependencies in the past I've been able to build the engine but then flutter run fail when trying to run an app.
Fixes https://github.com/flutter/flutter/issues/129307, also unblocks a feature that will eventually be needed for Scribe.
[*]`2.8.0` is the latest there, but I ran into an issue with dexing when I tried to upgrade, due to b/336164417, an AGP bug that had its fix backported to all >`8.0.0` versions, but we still support less than that so we will have to wait on that upgrade.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Android only for now but this avoids rebuilding a bunch of test targets that aren't necessary to do a `flutter run` with a custom engine.
I need to fill in some blanks for other platforms before landing.
This flag as originally introduced to see if it made the GN step slower. It did not do so in any measurable manner and the default was flipped to true. AFAICT, no one disables and its good to have tracelogs to debug GN slowdowns. Remove the flag.
Removes impeller_capture, impeller_use_prebuilt_impellerc, impeller_use_prebuilt_scenec, and impeller_trace_canvas.
Also removes setting these flags from ./flutter/tools/gn.
Closes https://github.com/flutter/flutter/issues/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:
```mermaid
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`][1] type to avoid awkward
string parsing/manipulation all over
- Replaced `gn_utils` (i.e. `targetsFromCommandLine` and friends) with
[an invocation of `gn desc`][2]
- 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):
```sh
# 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.
[1]:
https://github.com/flutter/engine/compare/main...matanlurey:engine:gn-label?expand=1#diff-ac008475ce9e209f4ecf6594b0cd48d12d86a1ffa0aa7d555375ff43b983eb2b
[2]:
https://github.com/flutter/engine/compare/main...matanlurey:engine:gn-label?expand=1#diff-ad116b8504dd0a500bf758a327c7052e5733a2b465d3295e35b74458d483276f
I started on a refactoring of
https://github.com/flutter/flutter/issues/147666, and was frustrated
with the JSON decoding of `gn desc --format=json`, so I used a similar
pattern from my micro-json library (https://pub.dev/packages/jsonut) and
encapsulated it as an extension type.
My favorite is the `JsonObject.map` function which replaces:
```dart
/// Construct a RunTarget from a JSON map.
factory RunTarget.fromJson(Map<String, Object> map) {
final List<String> errors = <String>[];
final String name = stringOfJson(map, _nameKey, errors)!;
final String id = stringOfJson(map, _idKey, errors)!;
final String targetPlatform =
stringOfJson(map, _targetPlatformKey, errors)!;
if (errors.isNotEmpty) {
throw FormatException('Failed to parse RunTarget: ${errors.join('\n')}');
}
return RunTarget._(name, id, targetPlatform);
}
```
... with:
```dart
/// Construct a RunTarget from a JSON map.
factory RunTarget.fromJson(Map<String, Object> map) {
return JsonObject(map).map((JsonObject json) => RunTarget._(
json.string(_nameKey),
json.string(_idKey),
json.string(_targetPlatformKey),
));
}
```
The size of the LTO build of the engine with the dylib compressed is as follows:
```sh
$ du -k libFlutter*
5236 libFlutter.dylib.tar.gz
4324 libFlutterSlimpeller.dylib.tar.gz
```
Sizes are in KiB. This represents a binary size reduction of 17.41% of the compressed artifacts. The compression ratios will likely differ based on the compression scheme.
Uncompressed, the sizes are:
```sh
$ du -k libFlutter*
16920 libFlutter.dylib
14044 libFlutterSlimpeller.dylib
```
This represents a binary size reduction of 16.99% which is in the same ballpark.
The really mucky bit was backing out the raster cache and persistent cache. I want to clean that up in a later patch so that those TUs are part of a separate submodule.
Opting out of Impeller will lead to a fatal log at startup saying the opt-out is disallowed.
Fixes https://github.com/flutter/flutter/issues/126606
This PR changes from using a different GN pool for each non-compiler tool, to using one GN pool for everything.
Additionally, since we are no longer linking remotely in any configuration, this PR uses the pool for linking.
ObjC and impellerc tasks also do not (yet) run on RBE, so they are also now governed by the pool.
Needs https://github.com/flutter/buildroot/pull/856
For illustrative purposes:
```sh
$ et build | grep '.*'
```
... should still get line-per-line status updates, but it does not without this patch.
It's hard to write tests because of global state, so I've declined to do so at the moment.
Closes https://github.com/flutter/flutter/issues/147903.
For https://github.com/flutter/flutter/issues/147750.
Depends on https://github.com/flutter/buildroot/pull/852.
This PR rolls forward reclient, the buildroot, and our RBE configs. The
new RBE configs do a few things. First they default the exec strategy to
`racing`. Second, they add a local wrapper script that passes the
correct value for `-fdebug-prefix-map` in both the local and remote
cases. Finally, they remove the canonicalization of the build's working
directory, so that the remote working directory will have the same name
as the local working directory. This solves the issue where the build
working directory contained a mysterious `set_by_reclient/a` component.
Previously, `Logger.test` was a leaky abstraction that stored a `@visibleForTesting` variable that was sometimes unused. It saved a few lines of code for tests, but doesn't seem necessary, so I swapped it out for a callback.
Closes https://github.com/flutter/flutter/issues/147894.
While doing this PR I realized we were basically passing `(bool verbose, Envrionment)` as a tuple around, so I just moved the concept _into_ `Environment` directly, and made the necessary code changes across the tool and tests.
To clarify, this does _not_ mimic the output of `ninja --verbose` _today_, because we also don't stream the output directly, and instead do terminal magic. Combined with a hypothetical fix for https://github.com/flutter/flutter/issues/147903, this would work exactly the same as before.
/cc @loic-sharma @johnmccutchan
Recent versions of clang-tidy have been segfaulting when processing a few of the engine source files. This patch skips over those errors so that the engine can roll Clang.
See https://github.com/flutter/flutter/issues/143178
This removes goma most places except for the arguments to the `gn`
script, which are referenced by recipes. This does not remove goma
capabilities from the GN build entirely. That requires changes in the
buildroot which have to be staged after this change.
https://github.com/flutter/engine/pull/52021 introduced a bug where `ci`
build could only be specified when also passing the `--verbose` flag.
This PR fixes the issue so that by default the `help` message will only
show `ci` builds when `--verbose` is passed to `help`, but the `ci`
builds can be specified even without the `--verbose` flag.
Adds a `Matcher` that can be used against a `List<ExecutedProcess>` to check if a command that matches a predicate was executed. Fails if no such command was found.
An alternative would be to take a single regex instead of a closure.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The `Command`s added to a `CommandRunner` don't automatically inherit
the `CommandRunner`'s usage line length limit. This PR does the
necessary plumbing. Help messages look nicer now.
The Dart commit https://dart-review.googlesource.com/c/sdk/+/361781 added dependencies upon packages meta and _fe_analyzer_shared to the kernel package. Path overrides for these need to be added to the const_finder package in flutter/engine.
This PR does two things. First, in many of the `ci` builds, LTO is
enabled by default. This is usually not what we want when doing local
builds, so this PR adds an `--lto` flag to the `build` command which is
disabled by default, causing `--no-lto` to be passed to GN. When `--lto`
is passed to the `build` command, `--lto` is passed to GN, which results
in the build using LTO.
Second, this PR eagerly parses the `--verbose` flag out of the command
line so that help messages can optionally show less information. In
particular, in this PR, `ci` and `web_test` builds are only displayed by
`help build` when `--verbose` is passed to the `help` command. There's
some extra text in the help message as well indicating that passing
`--verbose` to `help` will show more builds.
Reverts: flutter/engine#51229
Initiated by: gmackall
Reason for reverting: blocking engine->framework roll (I missed some framework code referencing the v1 embedding).
Original PR Author: gmackall
Reviewed By: {matanlurey, reidbaker}
This change reverts the following previous change:
Fixes https://github.com/flutter/flutter/issues/143531
Also fixes a random typo I found
~TODO to test this~ (no more todo):
-~test the framework against this as well, probably with a dummy PR changing the engine commit to my branch if this is possible~ not possible, made a best effort removal of framework code in https://github.com/flutter/flutter/pull/144726.
-~figure out if the old embedding is used in g3 at all~ removed all uses
-~figure out exactly what the ShimPluginRegistry/ShimRegistrar are doing, and if fully deleting them was right~ (see https://github.com/flutter/engine/pull/51229#issuecomment-1981757743)
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Addresses the notes I left on
https://github.com/flutter/engine/pull/51868. Mainly fixes an issue in
which the `build`, `query`, and `test` commands would fail if GN had not
yet been run for the requested configuration. Instead, in this PR, if
the build output directory doesn't exist yet, then it will run GN before
querying the targets.
Additionally, for the `build` command, if no targets are specified on
the command line, the PR uses the targets in the build config as the
default rather than all targets. `query` and `test` keep the same
behavior.
