From bf5f6177383ebd17a03b4c7ae3f67d59c1c9b53b Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 30 May 2024 16:01:55 -0700 Subject: [PATCH 1/2] Remove RBE crud compile_commands.json - Parsing a 21MB json string is not performant - RegExp replacing all occurances in said 21MB string is (~61ms) - Fixes #147767 --- .../lib/src/build_config_runner.dart | 29 ++++++++- .../test/build_config_runner_test.dart | 59 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index dffc8c8c1823a..5e3e8042db166 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -5,8 +5,9 @@ import 'dart:async'; import 'dart:convert'; import 'dart:ffi' as ffi; -import 'dart:io' as io show Directory, Process; +import 'dart:io' as io show Directory, File, Process; +import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:process_runner/process_runner.dart'; @@ -246,6 +247,7 @@ final class BuildRunner extends Runner { if (!await _runGn(eventHandler)) { return false; } + await _postGn(); } if (runNinja) { @@ -318,6 +320,31 @@ final class BuildRunner extends Runner { return result.ok; } + Future _postGn() async { + if (dryRun) { + return; + } + final Engine engine = Engine.fromSrcPath(engineSrcDir.path); + final Output? latest = engine.latestOutput(); + if (latest == null) { + return; + } + final io.File commandsFile = latest.compileCommandsJson; + if (!commandsFile.existsSync()) { + return; + } + final RegExp regex = RegExp(r'("command"\s*:\s*").*(\s\S*clang\+\+)'); + String contents = await commandsFile.readAsString(); + int matches = 0; + contents = contents.replaceAllMapped(regex, (Match match) { + matches += 1; + return '${match[1]}${match[2]!.trim()}'; + }); + if (matches > 0) { + await commandsFile.writeAsString(contents); + } + } + late final String _hostCpu = () { return switch (abi) { ffi.Abi.linuxArm64 || diff --git a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index bdfd50ad4f809..05d848aaeb892 100644 --- a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart @@ -10,6 +10,7 @@ import 'package:engine_build_configs/src/build_config.dart'; import 'package:engine_build_configs/src/build_config_runner.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:litetest/litetest.dart'; +import 'package:path/path.dart' as path; import 'package:platform/platform.dart'; import 'package:process_fakes/process_fakes.dart'; import 'package:process_runner/process_runner.dart'; @@ -577,6 +578,64 @@ void main() { } expect(caughtError, isTrue); }); + + test('GlobalBuildRunner trims RBE crud from compile_commands.json', () async { + final io.Directory emptyDir = + io.Directory.systemTemp.createTempSync('build_config_runner.test'); + try { + final io.Directory srcDir = io.Directory(path.join(emptyDir.path, 'src')) + ..createSync(recursive: true); + final io.Directory hostDebug = + io.Directory(path.join(srcDir.path, 'out', 'build_name')) + ..createSync(recursive: true); + final io.File file = + io.File(path.join(hostDebug.path, 'compile_commands.json')); + file.writeAsString(r''' +[ + { + "file": "../../flutter/assets/asset_manager.cc", + "directory": "/Users/flutter/src/engine/src/out/host_debug_unopt_arm64", + "command": "/Users/flutter/src/engine/src/flutter/buildtools/mac-arm64/reclient/rewrapper --cfg=/Users/flutter/src/engine/src/flutter/build/rbe/rewrapper-mac-arm64.cfg --exec_root=/Users/flutter/src/engine/src/ --remote_wrapper=../../flutter/build/rbe/remote_wrapper.sh --local_wrapper=../../flutter/build/rbe/local_wrapper.sh --labels=type=compile,compiler=clang,lang=cpp ../../flutter/buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/assets/assets.asset_manager.o.d -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 \"-DDART_LEGACY_API=[[deprecated]]\" -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -I../.. -Igen -I../../flutter/third_party/libcxx/include -I../../flutter/third_party/libcxxabi/include -I../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../flutter -fno-strict-aliasing -fstack-protector-all --target=arm64-apple-macos -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O0 -g2 -Wunreachable-code -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../flutter/prebuilts/SDKs/MacOSX14.0.sdk -mmacosx-version-min=10.14.0 -c ../../flutter/assets/asset_manager.cc -o obj/flutter/assets/assets.asset_manager.o" + }, + { + "file": "../../flutter/assets/directory_asset_bundle.cc", + "directory": "/Users/flutter/src/engine/src/out/host_debug_unopt_arm64", + "command": "/Users/flutter/src/engine/src/flutter/buildtools/mac-arm64/reclient/rewrapper --cfg=/Users/flutter/src/engine/src/flutter/build/rbe/rewrapper-mac-arm64.cfg --exec_root=/Users/flutter/src/engine/src/ --remote_wrapper=../../flutter/build/rbe/remote_wrapper.sh --local_wrapper=../../flutter/build/rbe/local_wrapper.sh --labels=type=compile,compiler=clang,lang=cpp ../../flutter/buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/assets/assets.directory_asset_bundle.o.d -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 \"-DDART_LEGACY_API=[[deprecated]]\" -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -I../.. -Igen -I../../flutter/third_party/libcxx/include -I../../flutter/third_party/libcxxabi/include -I../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../flutter -fno-strict-aliasing -fstack-protector-all --target=arm64-apple-macos -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O0 -g2 -Wunreachable-code -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../flutter/prebuilts/SDKs/MacOSX14.0.sdk -mmacosx-version-min=10.14.0 -c ../../flutter/assets/directory_asset_bundle.cc -o obj/flutter/assets/assets.directory_asset_bundle.o" + }, +] +'''); + final Build targetBuild = buildConfig.builds[0]; + final BuildRunner buildRunner = BuildRunner( + platform: FakePlatform(operatingSystem: Platform.linux), + processRunner: ProcessRunner( + // dryRun should not try to spawn any processes. + processManager: _fakeProcessManager(), + ), + abi: ffi.Abi.linuxX64, + engineSrcDir: srcDir, + build: targetBuild, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + await buildRunner.run(handler); + expect(file.readAsStringSync(), r''' +[ + { + "file": "../../flutter/assets/asset_manager.cc", + "directory": "/Users/flutter/src/engine/src/out/host_debug_unopt_arm64", + "command": "../../flutter/buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/assets/assets.asset_manager.o.d -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 \"-DDART_LEGACY_API=[[deprecated]]\" -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -I../.. -Igen -I../../flutter/third_party/libcxx/include -I../../flutter/third_party/libcxxabi/include -I../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../flutter -fno-strict-aliasing -fstack-protector-all --target=arm64-apple-macos -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O0 -g2 -Wunreachable-code -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../flutter/prebuilts/SDKs/MacOSX14.0.sdk -mmacosx-version-min=10.14.0 -c ../../flutter/assets/asset_manager.cc -o obj/flutter/assets/assets.asset_manager.o" + }, + { + "file": "../../flutter/assets/directory_asset_bundle.cc", + "directory": "/Users/flutter/src/engine/src/out/host_debug_unopt_arm64", + "command": "../../flutter/buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/assets/assets.directory_asset_bundle.o.d -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 \"-DDART_LEGACY_API=[[deprecated]]\" -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -I../.. -Igen -I../../flutter/third_party/libcxx/include -I../../flutter/third_party/libcxxabi/include -I../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../flutter -fno-strict-aliasing -fstack-protector-all --target=arm64-apple-macos -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O0 -g2 -Wunreachable-code -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../flutter/prebuilts/SDKs/MacOSX14.0.sdk -mmacosx-version-min=10.14.0 -c ../../flutter/assets/directory_asset_bundle.cc -o obj/flutter/assets/assets.directory_asset_bundle.o" + }, +] +'''); + } finally { + emptyDir.deleteSync(recursive: true); + } + }); } FakeProcessManager _fakeProcessManager({ From 5d9c41a7c16d760340bd076c1e385a02446fc735 Mon Sep 17 00:00:00 2001 From: John McDole Date: Fri, 31 May 2024 12:52:27 -0700 Subject: [PATCH 2/2] Address reviewer comments and maybe the linux test failure. --- .../lib/src/build_config_runner.dart | 17 +++++++++-------- .../test/build_config_runner_test.dart | 12 +++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index 5e3e8042db166..35d5d0e4a9e21 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -7,7 +7,6 @@ import 'dart:convert'; import 'dart:ffi' as ffi; import 'dart:io' as io show Directory, File, Process; -import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:process_runner/process_runner.dart'; @@ -324,21 +323,23 @@ final class BuildRunner extends Runner { if (dryRun) { return; } - final Engine engine = Engine.fromSrcPath(engineSrcDir.path); - final Output? latest = engine.latestOutput(); - if (latest == null) { - return; - } - final io.File commandsFile = latest.compileCommandsJson; + + final io.File commandsFile = io.File(p.join( + engineSrcDir.path, + 'out', + build.ninja.config, + 'compile_commands.json', + )); if (!commandsFile.existsSync()) { return; } + final RegExp regex = RegExp(r'("command"\s*:\s*").*(\s\S*clang\+\+)'); String contents = await commandsFile.readAsString(); int matches = 0; contents = contents.replaceAllMapped(regex, (Match match) { matches += 1; - return '${match[1]}${match[2]!.trim()}'; + return '${match[1]}${match[2]!.trim()}'; }); if (matches > 0) { await commandsFile.writeAsString(contents); diff --git a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index 05d848aaeb892..388400842f948 100644 --- a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart @@ -580,17 +580,17 @@ void main() { }); test('GlobalBuildRunner trims RBE crud from compile_commands.json', () async { - final io.Directory emptyDir = + final io.Directory emptyDir = io.Directory.systemTemp.createTempSync('build_config_runner.test'); try { - final io.Directory srcDir = io.Directory(path.join(emptyDir.path, 'src')) - ..createSync(recursive: true); + final io.Directory srcDir = io.Directory(path.join(emptyDir.path, 'src')); final io.Directory hostDebug = io.Directory(path.join(srcDir.path, 'out', 'build_name')) ..createSync(recursive: true); final io.File file = io.File(path.join(hostDebug.path, 'compile_commands.json')); - file.writeAsString(r''' + file.writeAsString( + r''' [ { "file": "../../flutter/assets/asset_manager.cc", @@ -603,7 +603,9 @@ void main() { "command": "/Users/flutter/src/engine/src/flutter/buildtools/mac-arm64/reclient/rewrapper --cfg=/Users/flutter/src/engine/src/flutter/build/rbe/rewrapper-mac-arm64.cfg --exec_root=/Users/flutter/src/engine/src/ --remote_wrapper=../../flutter/build/rbe/remote_wrapper.sh --local_wrapper=../../flutter/build/rbe/local_wrapper.sh --labels=type=compile,compiler=clang,lang=cpp ../../flutter/buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/assets/assets.directory_asset_bundle.o.d -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 \"-DDART_LEGACY_API=[[deprecated]]\" -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -I../.. -Igen -I../../flutter/third_party/libcxx/include -I../../flutter/third_party/libcxxabi/include -I../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../flutter -fno-strict-aliasing -fstack-protector-all --target=arm64-apple-macos -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O0 -g2 -Wunreachable-code -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../flutter/prebuilts/SDKs/MacOSX14.0.sdk -mmacosx-version-min=10.14.0 -c ../../flutter/assets/directory_asset_bundle.cc -o obj/flutter/assets/assets.directory_asset_bundle.o" }, ] -'''); +''', + flush: true, + ); final Build targetBuild = buildConfig.builds[0]; final BuildRunner buildRunner = BuildRunner( platform: FakePlatform(operatingSystem: Platform.linux),