mirror of
https://github.com/dart-lang/sdk
synced 2024-11-02 10:28:02 +00:00
f0ca213d60
This CL optimizes how CFE et al presubmits are run. In the examples below we'll that it takes the presubmit time from 31+ to ~13 seconds, from 31+ to ~20 seconds and from 30+ to ~19 seconds on a few simple cases and from 76+ to ~27 seconds in a case where files in both _fe_analyzer_shared, front_end, frontend_server and kernel are changed. Before this CL, if there was changes in both front_end and frontend_server for instance it would run one smoke-test for each. They would each technically only test things in their own directory, but they would do a lot of overlapping work, e.g. compiling frontend_server also compiles front_end; the startup cost of a script is done several times etc. The bulk of the change in this CL is thus to only run things once. Now, if there is a change in both front_end and frontend_server the python presubmit will still launch a script for each, but it's just a light-weight script that will take ~400 ms to run (on my machine) if it decides to not do anything. What it does is that it looks at the changed files, from that it will know which presubmits will be run and decide which of them will actually do the work - the rest will just exit and say "it will be tested by this other one". Furthermore it then tries to run only the smoke tests necessary. For instance, if you have only changed a test in front_end it will only run the spell checker (and only for that file). Note that this is not perfect and there can be cases where you should get a presubmit error but wont. For instance if you remove all content from the spellchecking dictionary file it should give you lots of spelling mistake errors, but it won't because it won't actually run the spell checker (as no files it should spell check was changed). Probably you have to actively try to cheat it though, so I don't see it as a big problem. Things will still be checked fully on the CI. Additionally * the generated messages will have trailing commas which speeds up formatting of the generated files (in the cases where the generated files will have to be checked). * the explicit creation testing tool will do the outline of everything, but only do the bodies of the changed files. * building the "ast model" only compiles the outline. Left to do: * If only changing a single test, for instance, it will only run the spell checker on that file, but launching the isolate its run in still takes ~7 seconds because it loads up other stuff too. Maybe we could have special entry points for cases where it only should run an otherwise simple test. * The presubmit in the sdk dir (not CFE related) doesn't do well with many (big) changed files and testing them for formatting errors can easily take 10+ seconds (see example below where it contributes ~5 seconds for instance). Maybe `dart format` could be made faster, or maybe the script should test more than one file at once. *Example runs before and after*: Change in a single test file in front_end ========================================= Now: ``` $ time git cl presubmit -v -f [I2024-01-25 09:46:08,391 187077 140400494405504 presubmit_support.py] Found 1 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py Presubmit checks took 11.5s to calculate. Python 3 presubmit checks passed. real 0m12.772s user 0m16.093s sys 0m2.146s ``` Before: ``` $ time git cl presubmit -v -f [I2024-01-25 10:07:08,519 200015 140338735470464 presubmit_support.py] Found 1 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py 28.3s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Presubmit checks took 30.0s to calculate. Python 3 presubmit checks passed. real 0m31.396s user 2m9.500s sys 0m11.559s ``` So from 31+ to ~13 seconds. --------------------------------------------------------------------- Change in a single test file and a single lib file in front_end =============================================================== Now: ``` $ time git cl presubmit -v -f Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py 15.9s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Presubmit checks took 18.0s to calculate. Python 3 presubmit checks passed. real 0m19.365s user 0m33.157s sys 0m5.049s ``` Before: ``` $ time git cl presubmit -v -f [I2024-01-25 10:08:36,277 200953 140133274818432 presubmit_support.py] Found 2 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py 27.9s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Presubmit checks took 30.0s to calculate. Python 3 presubmit checks passed. real 0m31.311s user 2m9.854s sys 0m11.898s ``` So from 31+ to ~20 seconds. --------------------------------------------------------------------- Change only the messages file in front_end (but with generated files not changing) ================================================================================== Now: ``` $ time git cl presubmit -v -f [I2024-01-25 09:53:02,823 190466 140548397250432 presubmit_support.py] Found 1 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py 15.6s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Presubmit checks took 17.0s to calculate. Python 3 presubmit checks passed. real 0m18.326s user 0m38.999s sys 0m4.530s ``` Before: ``` $ time git cl presubmit -v -f [I2024-01-25 10:10:04,431 201892 140717686302592 presubmit_support.py] Found 1 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/front_end/PRESUBMIT.py 28.0s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Presubmit checks took 29.2s to calculate. Python 3 presubmit checks passed. real 0m30.550s user 2m9.488s sys 0m11.689s ``` So from 30+ to ~19 seconds. --------------------------------------------------------------------- Change several files: ``` $ git diff --stat pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart | 4 ++-- pkg/_fe_analyzer_shared/lib/src/parser/listener.dart | 2 ++ pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart | 2 ++ pkg/front_end/lib/src/base/processed_options.dart | 2 ++ pkg/front_end/messages.yaml | 2 +- pkg/front_end/tool/dart_doctest_impl.dart | 2 ++ pkg/frontend_server/lib/compute_kernel.dart | 2 ++ pkg/kernel/lib/ast.dart | 2 ++ 8 files changed, 15 insertions(+), 3 deletions(-) ``` ==================== Now: ``` [I2024-01-25 09:57:53,270 193911 140320429016960 presubmit_support.py] Found 8 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/_fe_analyzer_shared/PRESUBMIT.py 17.8s to run CheckChangeOnCommit from [...]/sdk/pkg/_fe_analyzer_shared/PRESUBMIT.py. Running [...]/sdk/pkg/front_end/PRESUBMIT.py Running [...]/sdk/pkg/frontend_server/PRESUBMIT.py Running [...]/sdk/pkg/kernel/PRESUBMIT.py Presubmit checks took 25.3s to calculate. Python 3 presubmit checks passed. real 0m26.585s user 1m8.997s sys 0m8.742s ``` Worth noting here is that "sdk/PRESUBMIT.py" takes 5+ seconds here Before: ``` [I2024-01-25 10:11:39,863 203026 140202046494592 presubmit_support.py] Found 8 file(s). Running Python 3 presubmit commit checks ... Running [...]/sdk/PRESUBMIT.py Running [...]/sdk/pkg/_fe_analyzer_shared/PRESUBMIT.py 14.6s to run CheckChangeOnCommit from [...]/sdk/pkg/_fe_analyzer_shared/PRESUBMIT.py. Running [...]/sdk/pkg/front_end/PRESUBMIT.py 28.0s to run CheckChangeOnCommit from [...]/sdk/pkg/front_end/PRESUBMIT.py. Running [...]/sdk/pkg/frontend_server/PRESUBMIT.py 20.9s to run CheckChangeOnCommit from [...]/sdk/pkg/frontend_server/PRESUBMIT.py. Running [...]/sdk/pkg/kernel/PRESUBMIT.py Presubmit checks took 75.6s to calculate. Python 3 presubmit checks passed. real 1m16.870s user 3m48.784s sys 0m23.689s ``` So from 76+ to ~27 seconds. In response to https://github.com/dart-lang/sdk/issues/54665 Change-Id: I59a43f5009bba8c2fdcb5d3a843b4cb408499214 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348301 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com>
166 lines
6.6 KiB
Dart
166 lines
6.6 KiB
Dart
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
|
|
// for details. All rights reserved. Use of this source code is governed by a
|
|
// BSD-style license that can be found in the LICENSE file.
|
|
|
|
import 'dart:io';
|
|
|
|
import 'package:_fe_analyzer_shared/src/messages/severity.dart';
|
|
import 'package:expect/expect.dart' show Expect;
|
|
import 'package:front_end/src/api_prototype/compiler_options.dart';
|
|
import 'package:front_end/src/base/processed_options.dart';
|
|
import 'package:front_end/src/fasta/compiler_context.dart';
|
|
import 'package:front_end/src/fasta/dill/dill_target.dart';
|
|
import 'package:front_end/src/fasta/kernel/kernel_target.dart';
|
|
import 'package:front_end/src/fasta/ticker.dart';
|
|
import 'package:front_end/src/fasta/uri_translator.dart';
|
|
import 'package:kernel/kernel.dart';
|
|
import 'package:kernel/target/targets.dart';
|
|
import 'package:front_end/src/compute_platform_binaries_location.dart'
|
|
show computePlatformBinariesLocation;
|
|
import "package:vm/target/vm.dart" show VmTarget;
|
|
import 'utils/io_utils.dart' show computeRepoDirUri;
|
|
|
|
final Uri repoDir = computeRepoDirUri();
|
|
|
|
Set<String> allowlistedExternalDartFiles = {
|
|
"third_party/pkg/package_config/lib/package_config.dart",
|
|
"third_party/pkg/package_config/lib/package_config_types.dart",
|
|
"third_party/pkg/package_config/lib/src/discovery.dart",
|
|
"third_party/pkg/package_config/lib/src/errors.dart",
|
|
"third_party/pkg/package_config/lib/src/package_config_impl.dart",
|
|
"third_party/pkg/package_config/lib/src/package_config_io.dart",
|
|
"third_party/pkg/package_config/lib/src/package_config_json.dart",
|
|
"third_party/pkg/package_config/lib/src/package_config.dart",
|
|
"third_party/pkg/package_config/lib/src/packages_file.dart",
|
|
"third_party/pkg/package_config/lib/src/util.dart",
|
|
|
|
// TODO(johnniwinther): Fix to allow dependency of package:package_config.
|
|
"third_party/pkg/package_config/lib/src/util_io.dart",
|
|
|
|
// TODO(CFE-team): These files should not be included.
|
|
// The package isn't even in pubspec.yaml.
|
|
"pkg/meta/lib/meta.dart",
|
|
"pkg/meta/lib/meta_meta.dart",
|
|
};
|
|
|
|
/// Returns true on no errors and false if errors was found.
|
|
Future<bool> main() async {
|
|
Ticker ticker = new Ticker(isVerbose: false);
|
|
CompilerOptions compilerOptions = getOptions();
|
|
|
|
Uri packageConfigUri = repoDir.resolve(".dart_tool/package_config.json");
|
|
if (!new File.fromUri(packageConfigUri).existsSync()) {
|
|
throw "Couldn't find .dart_tool/package_config.json";
|
|
}
|
|
compilerOptions.packagesFileUri = packageConfigUri;
|
|
|
|
ProcessedOptions options = new ProcessedOptions(options: compilerOptions);
|
|
|
|
Uri frontendLibUri = repoDir.resolve("pkg/front_end/lib/");
|
|
List<FileSystemEntity> entities =
|
|
new Directory.fromUri(frontendLibUri).listSync(recursive: true);
|
|
for (FileSystemEntity entity in entities) {
|
|
if (entity is File && entity.path.endsWith(".dart")) {
|
|
options.inputs.add(entity.uri);
|
|
}
|
|
}
|
|
|
|
List<Uri> result = await CompilerContext.runWithOptions<List<Uri>>(options,
|
|
(CompilerContext c) async {
|
|
UriTranslator uriTranslator = await c.options.getUriTranslator();
|
|
DillTarget dillTarget =
|
|
new DillTarget(ticker, uriTranslator, c.options.target);
|
|
KernelTarget kernelTarget =
|
|
new KernelTarget(c.fileSystem, false, dillTarget, uriTranslator);
|
|
Uri? platform = c.options.sdkSummary;
|
|
if (platform != null) {
|
|
var bytes = new File.fromUri(platform).readAsBytesSync();
|
|
var platformComponent = loadComponentFromBytes(bytes);
|
|
dillTarget.loader
|
|
.appendLibraries(platformComponent, byteCount: bytes.length);
|
|
}
|
|
|
|
kernelTarget.setEntryPoints(c.options.inputs);
|
|
dillTarget.buildOutlines();
|
|
await kernelTarget.loader.buildOutlines();
|
|
return new List<Uri>.from(c.dependencies);
|
|
});
|
|
|
|
Set<Uri> otherDartUris = new Set<Uri>();
|
|
Set<Uri> otherNonDartUris = new Set<Uri>();
|
|
Set<Uri> frontEndUris = new Set<Uri>();
|
|
Set<Uri> kernelUris = new Set<Uri>();
|
|
Set<Uri> feAnalyzerSharedUris = new Set<Uri>();
|
|
Set<Uri> dartPlatformUris = new Set<Uri>();
|
|
Uri kernelUri = repoDir.resolve("pkg/kernel/");
|
|
Uri feAnalyzerSharedUri = repoDir.resolve("pkg/_fe_analyzer_shared/");
|
|
Uri platformUri1 = repoDir.resolve("sdk/lib/");
|
|
Uri platformUri2 = repoDir.resolve("runtime/lib/");
|
|
Uri platformUri3 = repoDir.resolve("runtime/bin/");
|
|
for (Uri uri in result) {
|
|
if (uri.toString().startsWith(frontendLibUri.toString())) {
|
|
frontEndUris.add(uri);
|
|
} else if (uri.toString().startsWith(kernelUri.toString())) {
|
|
kernelUris.add(uri);
|
|
} else if (uri.toString().startsWith(feAnalyzerSharedUri.toString())) {
|
|
feAnalyzerSharedUris.add(uri);
|
|
} else if (uri.toString().startsWith(platformUri1.toString()) ||
|
|
uri.toString().startsWith(platformUri2.toString()) ||
|
|
uri.toString().startsWith(platformUri3.toString())) {
|
|
dartPlatformUris.add(uri);
|
|
} else if (uri.toString().endsWith(".dart")) {
|
|
otherDartUris.add(uri);
|
|
} else {
|
|
otherNonDartUris.add(uri);
|
|
}
|
|
}
|
|
|
|
// * Everything in frontEndUris is okay --- the frontend can import itself.
|
|
// * Everything in kernel is okay --- the frontend is allowed to
|
|
// import package:kernel.
|
|
// * For other entries, remove allowlisted entries.
|
|
// * Everything else is an error.
|
|
|
|
// Remove white-listed non-dart files.
|
|
otherNonDartUris.remove(packageConfigUri);
|
|
otherNonDartUris.remove(repoDir.resolve("sdk/lib/libraries.json"));
|
|
otherNonDartUris.remove(repoDir.resolve(".dart_tool/package_config.json"));
|
|
|
|
// Remove white-listed dart files.
|
|
for (String s in allowlistedExternalDartFiles) {
|
|
otherDartUris.remove(repoDir.resolve(s));
|
|
}
|
|
|
|
if (otherNonDartUris.isNotEmpty || otherDartUris.isNotEmpty) {
|
|
print("The following files was imported without being allowlisted:");
|
|
for (Uri uri in otherNonDartUris) {
|
|
print(" - $uri");
|
|
}
|
|
for (Uri uri in otherDartUris) {
|
|
print(" - $uri");
|
|
}
|
|
exitCode = 1;
|
|
return false;
|
|
}
|
|
return true;
|
|
}
|
|
|
|
CompilerOptions getOptions() {
|
|
// Compile sdk because when this is run from a lint it uses the checked-in sdk
|
|
// and we might not have a suitable compiled platform.dill file.
|
|
Uri sdkRoot = computePlatformBinariesLocation(forceBuildDir: true);
|
|
CompilerOptions options = new CompilerOptions()
|
|
..sdkRoot = sdkRoot
|
|
..compileSdk = true
|
|
..target = new VmTarget(new TargetFlags())
|
|
..librariesSpecificationUri = repoDir.resolve("sdk/lib/libraries.json")
|
|
..omitPlatform = true
|
|
..onDiagnostic = (DiagnosticMessage message) {
|
|
if (message.severity == Severity.error) {
|
|
Expect.fail(
|
|
"Unexpected error: ${message.plainTextFormatted.join('\n')}");
|
|
}
|
|
}
|
|
..environmentDefines = const {};
|
|
return options;
|
|
}
|