Improve AOT snapshot input verification + cleanup (#17144)
Bugfix: Moves AOT snapshot input verification past where the last input
is added to the inputs list.
Cleanup:
* Extracts _isValidAotPlatform method.
* Moves non-platform-specific logic to the top.
* Moves variable declaration closer to first use, and inlines to a
narrower scope where possible.
This relands #17136, which was reverted in #17142 due to breakage in
on-device iOS debug builds.
diff --git a/packages/flutter_tools/lib/src/base/build.dart b/packages/flutter_tools/lib/src/base/build.dart
index 0f32839..3a2f609 100644
--- a/packages/flutter_tools/lib/src/base/build.dart
+++ b/packages/flutter_tools/lib/src/base/build.dart
@@ -264,36 +264,16 @@
@required bool preferSharedLibrary,
List<String> extraGenSnapshotOptions: const <String>[],
}) async {
- if (!(platform == TargetPlatform.android_arm ||
- platform == TargetPlatform.android_arm64 ||
- platform == TargetPlatform.ios)) {
+ if (!_isValidAotPlatform(platform)) {
printError('${getNameForTargetPlatform(platform)} does not support AOT compilation.');
return -2;
}
- final Directory outputDir = fs.directory(outputPath);
- outputDir.createSync(recursive: true);
-
- final String vmSnapshotData = fs.path.join(outputDir.path, 'vm_snapshot_data');
- final String isolateSnapshotData = fs.path.join(outputDir.path, 'isolate_snapshot_data');
- final String depfilePath = fs.path.join(outputDir.path, 'snapshot.d');
- final String assembly = fs.path.join(outputDir.path, 'snapshot_assembly.S');
- final String assemblyO = fs.path.join(outputDir.path, 'snapshot_assembly.o');
- final String assemblySo = fs.path.join(outputDir.path, 'app.so');
- final bool compileToSharedLibrary =
- preferSharedLibrary && androidSdk.ndkCompiler != null;
-
+ final bool compileToSharedLibrary = preferSharedLibrary && androidSdk.ndkCompiler != null;
if (preferSharedLibrary && !compileToSharedLibrary) {
- printStatus(
- 'Could not find NDK compiler. Not building in shared library mode');
+ printStatus('Could not find NDK compiler. Not building in shared library mode.');
}
- final String vmEntryPoints = artifacts.getArtifactPath(Artifact.dartVmEntryPointsTxt, platform, buildMode);
- assert(vmEntryPoints != null);
-
- final String ioEntryPoints = artifacts.getArtifactPath(Artifact.dartIoEntriesTxt, platform, buildMode);
- assert(ioEntryPoints != null);
-
final PackageMap packageMap = new PackageMap(packagesPath);
final String packageMapError = packageMap.checkValid();
if (packageMapError != null) {
@@ -301,19 +281,19 @@
return -3;
}
+ final Directory outputDir = fs.directory(outputPath);
+ outputDir.createSync(recursive: true);
+
final String skyEnginePkg = _getPackagePath(packageMap, 'sky_engine');
final String uiPath = fs.path.join(skyEnginePkg, 'lib', 'ui', 'ui.dart');
final String vmServicePath = fs.path.join(skyEnginePkg, 'sdk_ext', 'vmservice_io.dart');
- final List<String> inputPaths = <String>[vmEntryPoints, ioEntryPoints, uiPath, vmServicePath, mainPath];
+ final List<String> inputPaths = <String>[uiPath, vmServicePath, mainPath];
final Set<String> outputPaths = new Set<String>();
- final Iterable<String> missingInputs = inputPaths.where((String p) => !fs.isFileSync(p));
- if (missingInputs.isNotEmpty) {
- printError('Missing input files: $missingInputs');
- return -4;
- }
-
+ final String vmSnapshotData = fs.path.join(outputDir.path, 'vm_snapshot_data');
+ final String isolateSnapshotData = fs.path.join(outputDir.path, 'isolate_snapshot_data');
+ final String depfilePath = fs.path.join(outputDir.path, 'snapshot.d');
final List<String> genSnapshotArgs = <String>[
'--vm_snapshot_data=$vmSnapshotData',
'--isolate_snapshot_data=$isolateSnapshotData',
@@ -321,42 +301,56 @@
'--url_mapping=dart:vmservice_io,$vmServicePath',
'--dependencies=$depfilePath',
];
-
- if ((extraGenSnapshotOptions != null) && extraGenSnapshotOptions.isNotEmpty) {
- printTrace('Extra gen-snapshot options: $extraGenSnapshotOptions');
+ if (previewDart2) {
+ genSnapshotArgs.addAll(<String>[
+ '--reify-generic-functions',
+ '--strong',
+ ]);
+ }
+ if (buildMode != BuildMode.release) {
+ genSnapshotArgs.addAll(<String>[
+ '--no-checked',
+ '--conditional_directives',
+ ]);
+ }
+ if (extraGenSnapshotOptions != null && extraGenSnapshotOptions.isNotEmpty) {
+ printTrace('Extra gen_snapshot options: $extraGenSnapshotOptions');
genSnapshotArgs.addAll(extraGenSnapshotOptions);
}
final bool interpreter = _isInterpreted(platform, buildMode);
if (!interpreter) {
+ final String vmEntryPoints = artifacts.getArtifactPath(Artifact.dartVmEntryPointsTxt, platform, buildMode);
+ final String ioEntryPoints = artifacts.getArtifactPath(Artifact.dartIoEntriesTxt, platform, buildMode);
genSnapshotArgs.add('--embedder_entry_points_manifest=$vmEntryPoints');
genSnapshotArgs.add('--embedder_entry_points_manifest=$ioEntryPoints');
+ inputPaths..addAll(<String>[vmEntryPoints, ioEntryPoints]);
}
- // iOS symbols used to load snapshot data in the engine.
+ // iOS snapshot generated files, compiled object files.
const String kVmSnapshotData = 'kDartVmSnapshotData';
const String kIsolateSnapshotData = 'kDartIsolateSnapshotData';
-
- // iOS snapshot generated files, compiled object files.
final String kVmSnapshotDataO = fs.path.join(outputDir.path, '$kVmSnapshotData.o');
final String kIsolateSnapshotDataO = fs.path.join(outputDir.path, '$kIsolateSnapshotData.o');
+ // Compile-to-assembly outputs.
+ final String assembly = fs.path.join(outputDir.path, 'snapshot_assembly.S');
+ final String assemblyO = fs.path.join(outputDir.path, 'snapshot_assembly.o');
+ final String assemblySo = fs.path.join(outputDir.path, 'app.so');
+
switch (platform) {
case TargetPlatform.android_arm:
case TargetPlatform.android_arm64:
case TargetPlatform.android_x64:
case TargetPlatform.android_x86:
- final String vmSnapshotInstructions = fs.path.join(outputDir.path, 'vm_snapshot_instr');
- final String isolateSnapshotInstructions = fs.path.join(outputDir.path, 'isolate_snapshot_instr');
if (compileToSharedLibrary) {
outputPaths.add(assemblySo);
genSnapshotArgs.add('--snapshot_kind=app-aot-assembly');
genSnapshotArgs.add('--assembly=$assembly');
} else {
- outputPaths.addAll(<String>[
- vmSnapshotData,
- isolateSnapshotData,
- ]);
+ final String vmSnapshotInstructions = fs.path.join(outputDir.path, 'vm_snapshot_instr');
+ final String isolateSnapshotInstructions = fs.path.join(outputDir.path, 'isolate_snapshot_instr');
+ outputPaths.addAll(<String>[vmSnapshotData, isolateSnapshotData]);
genSnapshotArgs.addAll(<String>[
'--snapshot_kind=app-aot-blobs',
'--vm_snapshot_instructions=$vmSnapshotInstructions',
@@ -371,15 +365,9 @@
}
break;
case TargetPlatform.ios:
- final String snapshotDartIOS = artifacts.getArtifactPath(Artifact.snapshotDart, platform, buildMode);
- inputPaths.add(snapshotDartIOS);
if (interpreter) {
genSnapshotArgs.add('--snapshot_kind=core');
- genSnapshotArgs.add(snapshotDartIOS);
- outputPaths.addAll(<String>[
- kVmSnapshotDataO,
- kIsolateSnapshotDataO,
- ]);
+ outputPaths.addAll(<String>[kVmSnapshotDataO, kIsolateSnapshotDataO]);
} else {
genSnapshotArgs.add('--snapshot_kind=app-aot-assembly');
genSnapshotArgs.add('--assembly=$assembly');
@@ -394,13 +382,22 @@
assert(false);
}
- if (buildMode != BuildMode.release) {
- genSnapshotArgs.addAll(<String>[
- '--no-checked',
- '--conditional_directives',
- ]);
+ if (interpreter) {
+ final String snapshotDartIOS = artifacts.getArtifactPath(Artifact.snapshotDart, platform, buildMode);
+ inputPaths.add(snapshotDartIOS);
+ genSnapshotArgs.add(snapshotDartIOS);
+ } else {
+ genSnapshotArgs.add(mainPath);
}
+ // Verify that all required inputs exist.
+ final Iterable<String> missingInputs = inputPaths.where((String p) => !fs.isFileSync(p));
+ if (missingInputs.isNotEmpty) {
+ printError('Missing input files: $missingInputs from $inputPaths');
+ return -4;
+ }
+
+ // If inputs and outputs have not changed since last run, skip the build.
final String fingerprintPath = '$depfilePath.fingerprint';
final SnapshotType snapshotType = new SnapshotType(platform, buildMode);
if (!await _isBuildRequired(snapshotType, outputPaths, depfilePath, mainPath, fingerprintPath)) {
@@ -408,14 +405,6 @@
return 0;
}
- if (previewDart2) {
- genSnapshotArgs.addAll(<String>[
- '--reify-generic-functions',
- '--strong',
- ]);
- }
- genSnapshotArgs.add(mainPath);
-
final int genSnapshotExitCode = await genSnapshot.run(
snapshotType: new SnapshotType(platform, buildMode),
packagesPath: packageMap.packagesPath,
@@ -436,12 +425,10 @@
if (platform == TargetPlatform.ios) {
printStatus('Building App.framework...');
- final String kVmSnapshotDataC = fs.path.join(outputDir.path, '$kVmSnapshotData.c');
- final String kIsolateSnapshotDataC = fs.path.join(outputDir.path, '$kIsolateSnapshotData.c');
-
const List<String> commonBuildOptions = const <String>['-arch', 'arm64', '-miphoneos-version-min=8.0'];
-
if (interpreter) {
+ final String kVmSnapshotDataC = fs.path.join(outputDir.path, '$kVmSnapshotData.c');
+ final String kIsolateSnapshotDataC = fs.path.join(outputDir.path, '$kIsolateSnapshotData.c');
await fs.file(vmSnapshotData).rename(fs.path.join(outputDir.path, kVmSnapshotData));
await fs.file(isolateSnapshotData).rename(fs.path.join(outputDir.path, kIsolateSnapshotData));
@@ -497,6 +484,14 @@
return 0;
}
+ bool _isValidAotPlatform(TargetPlatform platform) {
+ return const <TargetPlatform>[
+ TargetPlatform.android_arm,
+ TargetPlatform.android_arm64,
+ TargetPlatform.ios,
+ ].contains(platform);
+ }
+
/// Returns true if the specified platform and build mode require running in interpreted mode.
bool _isInterpreted(TargetPlatform platform, BuildMode buildMode) {
return platform == TargetPlatform.ios && buildMode == BuildMode.debug;
diff --git a/packages/flutter_tools/test/base/build_test.dart b/packages/flutter_tools/test/base/build_test.dart
index ca5e764..e58c87f 100644
--- a/packages/flutter_tools/test/base/build_test.dart
+++ b/packages/flutter_tools/test/base/build_test.dart
@@ -662,13 +662,12 @@
'--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
'--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
'--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
- '--snapshot_kind=core',
- 'snapshot.dart',
- '--no-checked',
- '--conditional_directives',
'--reify-generic-functions',
'--strong',
- 'main.dill',
+ '--no-checked',
+ '--conditional_directives',
+ '--snapshot_kind=core',
+ 'snapshot.dart',
]);
}, overrides: contextOverrides);
@@ -704,14 +703,14 @@
'--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
'--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
'--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
+ '--reify-generic-functions',
+ '--strong',
+ '--no-checked',
+ '--conditional_directives',
'--embedder_entry_points_manifest=$kVmEntrypoints',
'--embedder_entry_points_manifest=$kIoEntries',
'--snapshot_kind=app-aot-assembly',
'--assembly=${fs.path.join(outputPath, 'snapshot_assembly.S')}',
- '--no-checked',
- '--conditional_directives',
- '--reify-generic-functions',
- '--strong',
'main.dill',
]);
}, overrides: contextOverrides);
@@ -748,12 +747,12 @@
'--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
'--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
'--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
+ '--reify-generic-functions',
+ '--strong',
'--embedder_entry_points_manifest=$kVmEntrypoints',
'--embedder_entry_points_manifest=$kIoEntries',
'--snapshot_kind=app-aot-assembly',
'--assembly=${fs.path.join(outputPath, 'snapshot_assembly.S')}',
- '--reify-generic-functions',
- '--strong',
'main.dill',
]);
}, overrides: contextOverrides);
