Skip to content

Commit cec2400

Browse files
dcharkesdcharkes
authored
[native_assets] Stop running link hooks in JIT mode (#151534)
Stop running link hooks in debug mode. Rationale: link hooks only get access to tree-shaking info in release builds, so they can't do anything meaningful in debug builds. Debug builds should be fast as development cycle, so running less is better. More details: * dart-lang/native#1252 Also: rolls packages to latest versions. ## Implementation details The decision whether linking is enabled is made as follows: * For normal builds `build_info.dart::BuildMode` is used to determine whether Dart is compiled in JIT or AOT mode. * Testers always run in JIT, so no linking. * Native asset dry runs only run for JIT builds (e.g only when hot reload and hot restart are enabled). ## Testing The integration test is updated to output an asset for linking if `BuildConfig.linkingEnabled` is true, and to output an asset for bundling directly if linking is not enabled.
1 parent dddea4d commit cec2400

14 files changed

Lines changed: 636 additions & 631 deletions

File tree

‎dev/integration_tests/link_hook/hook/build.dart‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,22 @@ import 'package:native_toolchain_c/native_toolchain_c.dart';
99

1010
void main(List<String> args) async {
1111
await build(args, (BuildConfig config, BuildOutput output) async {
12+
final String assetName;
13+
if (config.linkingEnabled) {
14+
// The link hook will be run. So emit an asset with a name that is
15+
// not used, so that the link hook can rename it.
16+
// This will ensure the test fails if the link-hooks are not run
17+
// while being reported that linking is enabled.
18+
assetName = 'some_asset_name_that_is_not_used';
19+
} else {
20+
// The link hook will not be run, so immediately emit an asset for
21+
// bundling.
22+
assetName = '${config.packageName}_bindings_generated.dart';
23+
}
1224
final String packageName = config.packageName;
1325
final CBuilder cbuilder = CBuilder.library(
1426
name: packageName,
15-
assetName: 'some_asset_name_that_is_not_used',
27+
assetName: assetName,
1628
sources: <String>[
1729
'src/$packageName.c',
1830
],
@@ -27,10 +39,10 @@ void main(List<String> args) async {
2739
..onRecord.listen((LogRecord record) => print(record.message)),
2840
);
2941
output.addDependencies(outputCatcher.dependencies);
30-
// Send the asset to hook/link.dart.
42+
// Send the asset to hook/link.dart or immediately for bundling.
3143
output.addAsset(
3244
outputCatcher.assets.single,
33-
linkInPackage: 'link_hook',
45+
linkInPackage: config.linkingEnabled ? 'link_hook' : null,
3446
);
3547
});
3648
}

‎dev/integration_tests/link_hook/pubspec.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ environment:
88
dependencies:
99
cli_config: 0.2.0
1010
logging: 1.2.0
11-
native_assets_cli: 0.6.1
12-
native_toolchain_c: 0.5.0
11+
native_assets_cli: 0.7.0
12+
native_toolchain_c: 0.5.1
1313

1414
args: 2.5.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
1515
async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
@@ -67,4 +67,4 @@ dev_dependencies:
6767
webkit_inspection_protocol: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
6868
yaml_edit: 2.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
6969

70-
# PUBSPEC CHECKSUM: fe47
70+
# PUBSPEC CHECKSUM: 0248

‎packages/flutter_tools/lib/src/isolated/native_assets/android/native_assets.dart‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,8 @@ Future<Iterable<KernelAsset>> dryRunNativeAssetsAndroidInternal(
5757
includeParentEnvironment: true,
5858
);
5959
ensureNativeAssetsBuildDryRunSucceed(buildDryRunResult);
60-
final LinkDryRunResult linkDryRunResult = await buildRunner.linkDryRun(
61-
linkModePreference: LinkModePreferenceImpl.dynamic,
62-
targetOS: targetOS,
63-
workingDirectory: projectUri,
64-
includeParentEnvironment: true,
65-
buildDryRunResult: buildDryRunResult,
66-
);
67-
ensureNativeAssetsLinkDryRunSucceed(linkDryRunResult);
68-
final List<AssetImpl> nativeAssets = <AssetImpl>[
69-
...buildDryRunResult.assets,
70-
...linkDryRunResult.assets,
71-
];
60+
// No link hooks in JIT mode.
61+
final List<AssetImpl> nativeAssets = buildDryRunResult.assets;
7262
ensureNoLinkModeStatic(nativeAssets);
7363
globals.logger.printTrace('Dry running native assets for $targetOS done.');
7464
final Map<AssetImpl, KernelAsset> assetTargetLocations =
@@ -102,6 +92,7 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)>
10292
final List<Target> targets = androidArchs.map(_getNativeTarget).toList();
10393
final BuildModeImpl buildModeCli =
10494
nativeAssetsBuildMode(buildMode);
95+
final bool linkingEnabled = buildModeCli == BuildModeImpl.release;
10596

10697
globals.logger
10798
.printTrace('Building native assets for $targets $buildModeCli.');
@@ -116,23 +107,26 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)>
116107
includeParentEnvironment: true,
117108
cCompilerConfig: await buildRunner.ndkCCompilerConfigImpl,
118109
targetAndroidNdkApi: targetAndroidNdkApi,
110+
linkingEnabled: linkingEnabled,
119111
);
120112
ensureNativeAssetsBuildSucceed(buildResult);
121113
nativeAssets.addAll(buildResult.assets);
122114
dependencies.addAll(buildResult.dependencies);
123-
final LinkResult linkResult = await buildRunner.link(
124-
linkModePreference: LinkModePreferenceImpl.dynamic,
125-
target: target,
126-
buildMode: buildModeCli,
127-
workingDirectory: projectUri,
128-
includeParentEnvironment: true,
129-
cCompilerConfig: await buildRunner.ndkCCompilerConfigImpl,
130-
targetAndroidNdkApi: targetAndroidNdkApi,
131-
buildResult: buildResult,
132-
);
133-
ensureNativeAssetsLinkSucceed(linkResult);
134-
nativeAssets.addAll(linkResult.assets);
135-
dependencies.addAll(linkResult.dependencies);
115+
if (linkingEnabled) {
116+
final LinkResult linkResult = await buildRunner.link(
117+
linkModePreference: LinkModePreferenceImpl.dynamic,
118+
target: target,
119+
buildMode: buildModeCli,
120+
workingDirectory: projectUri,
121+
includeParentEnvironment: true,
122+
cCompilerConfig: await buildRunner.ndkCCompilerConfigImpl,
123+
targetAndroidNdkApi: targetAndroidNdkApi,
124+
buildResult: buildResult,
125+
);
126+
ensureNativeAssetsLinkSucceed(linkResult);
127+
nativeAssets.addAll(linkResult.assets);
128+
dependencies.addAll(linkResult.dependencies);
129+
}
136130
}
137131
ensureNoLinkModeStatic(nativeAssets);
138132
globals.logger.printTrace('Building native assets for $targets done.');

‎packages/flutter_tools/lib/src/isolated/native_assets/ios/native_assets.dart‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,8 @@ Future<Iterable<KernelAsset>> dryRunNativeAssetsIOSInternal(
5555
includeParentEnvironment: true,
5656
);
5757
ensureNativeAssetsBuildDryRunSucceed(buildDryRunResult);
58-
final LinkDryRunResult linkDryRunResult = await buildRunner.linkDryRun(
59-
linkModePreference: LinkModePreferenceImpl.dynamic,
60-
targetOS: targetOS,
61-
workingDirectory: projectUri,
62-
includeParentEnvironment: true,
63-
buildDryRunResult: buildDryRunResult,
64-
);
65-
ensureNativeAssetsLinkDryRunSucceed(linkDryRunResult);
66-
final List<AssetImpl> nativeAssets = <AssetImpl>[
67-
...buildDryRunResult.assets,
68-
...linkDryRunResult.assets,
69-
];
58+
// No link hooks in JIT.
59+
final List<AssetImpl> nativeAssets = buildDryRunResult.assets;
7060
ensureNoLinkModeStatic(nativeAssets);
7161
globals.logger.printTrace('Dry running native assets for $targetOS done.');
7262
return _assetTargetLocations(nativeAssets).values;
@@ -90,6 +80,7 @@ Future<List<Uri>> buildNativeAssetsIOS({
9080

9181
final List<Target> targets = darwinArchs.map(_getNativeTarget).toList();
9282
final BuildModeImpl buildModeCli = nativeAssetsBuildMode(buildMode);
83+
final bool linkingEnabled = buildModeCli == BuildModeImpl.release;
9384

9485
const OSImpl targetOS = OSImpl.iOS;
9586
final Uri buildUri = nativeAssetsBuildUri(projectUri, targetOS);
@@ -109,25 +100,28 @@ Future<List<Uri>> buildNativeAssetsIOS({
109100
cCompilerConfig: await buildRunner.cCompilerConfig,
110101
// TODO(dcharkes): Fetch minimum iOS version from somewhere. https://github.com/flutter/flutter/issues/145104
111102
targetIOSVersion: 12,
103+
linkingEnabled: linkingEnabled,
112104
);
113105
ensureNativeAssetsBuildSucceed(buildResult);
114106
nativeAssets.addAll(buildResult.assets);
115107
dependencies.addAll(buildResult.dependencies);
116-
final LinkResult linkResult = await buildRunner.link(
117-
linkModePreference: LinkModePreferenceImpl.dynamic,
118-
target: target,
119-
targetIOSSdkImpl: iosSdk,
120-
buildMode: buildModeCli,
121-
workingDirectory: projectUri,
122-
includeParentEnvironment: true,
123-
cCompilerConfig: await buildRunner.cCompilerConfig,
124-
buildResult: buildResult,
125-
// TODO(dcharkes): Fetch minimum iOS version from somewhere. https://github.com/flutter/flutter/issues/145104
126-
targetIOSVersion: 12,
127-
);
128-
ensureNativeAssetsLinkSucceed(linkResult);
129-
nativeAssets.addAll(linkResult.assets);
130-
dependencies.addAll(linkResult.dependencies);
108+
if (linkingEnabled) {
109+
final LinkResult linkResult = await buildRunner.link(
110+
linkModePreference: LinkModePreferenceImpl.dynamic,
111+
target: target,
112+
targetIOSSdkImpl: iosSdk,
113+
buildMode: buildModeCli,
114+
workingDirectory: projectUri,
115+
includeParentEnvironment: true,
116+
cCompilerConfig: await buildRunner.cCompilerConfig,
117+
buildResult: buildResult,
118+
// TODO(dcharkes): Fetch minimum iOS version from somewhere. https://github.com/flutter/flutter/issues/145104
119+
targetIOSVersion: 12,
120+
);
121+
ensureNativeAssetsLinkSucceed(linkResult);
122+
nativeAssets.addAll(linkResult.assets);
123+
dependencies.addAll(linkResult.dependencies);
124+
}
131125
}
132126
ensureNoLinkModeStatic(nativeAssets);
133127
globals.logger.printTrace('Building native assets for $targets done.');

‎packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,8 @@ Future<Iterable<KernelAsset>> dryRunNativeAssetsMacOSInternal(
5959
includeParentEnvironment: true,
6060
);
6161
ensureNativeAssetsBuildDryRunSucceed(buildDryRunResult);
62-
final LinkDryRunResult linkDryRunResult = await buildRunner.linkDryRun(
63-
linkModePreference: LinkModePreferenceImpl.dynamic,
64-
targetOS: targetOS,
65-
workingDirectory: projectUri,
66-
includeParentEnvironment: true,
67-
buildDryRunResult: buildDryRunResult,
68-
);
69-
ensureNativeAssetsLinkDryRunSucceed(linkDryRunResult);
70-
final List<AssetImpl> nativeAssets = <AssetImpl>[
71-
...buildDryRunResult.assets,
72-
...linkDryRunResult.assets,
73-
];
62+
// No link hooks in JIT mode.
63+
final List<AssetImpl> nativeAssets = buildDryRunResult.assets;
7464
ensureNoLinkModeStatic(nativeAssets);
7565
globals.logger.printTrace('Dry running native assets for $targetOS done.');
7666
final Uri? absolutePath = flutterTester ? buildUri : null;
@@ -115,6 +105,7 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsMacOS({
115105
: <Target>[Target.current];
116106
final BuildModeImpl buildModeCli =
117107
nativeAssetsBuildMode(buildMode);
108+
final bool linkingEnabled = buildModeCli == BuildModeImpl.release;
118109

119110
globals.logger
120111
.printTrace('Building native assets for $targets $buildModeCli.');
@@ -130,24 +121,27 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsMacOS({
130121
cCompilerConfig: await buildRunner.cCompilerConfig,
131122
// TODO(dcharkes): Fetch minimum MacOS version from somewhere. https://github.com/flutter/flutter/issues/145104
132123
targetMacOSVersion: 13,
124+
linkingEnabled: linkingEnabled,
133125
);
134126
ensureNativeAssetsBuildSucceed(buildResult);
135127
nativeAssets.addAll(buildResult.assets);
136128
dependencies.addAll(buildResult.dependencies);
137-
final LinkResult linkResult = await buildRunner.link(
138-
linkModePreference: LinkModePreferenceImpl.dynamic,
139-
target: target,
140-
buildMode: buildModeCli,
141-
workingDirectory: projectUri,
142-
includeParentEnvironment: true,
143-
cCompilerConfig: await buildRunner.cCompilerConfig,
144-
buildResult: buildResult,
145-
// TODO(dcharkes): Fetch minimum MacOS version from somewhere. https://github.com/flutter/flutter/issues/145104
146-
targetMacOSVersion: 13,
147-
);
148-
ensureNativeAssetsLinkSucceed(linkResult);
149-
nativeAssets.addAll(linkResult.assets);
150-
dependencies.addAll(linkResult.dependencies);
129+
if (linkingEnabled) {
130+
final LinkResult linkResult = await buildRunner.link(
131+
linkModePreference: LinkModePreferenceImpl.dynamic,
132+
target: target,
133+
buildMode: buildModeCli,
134+
workingDirectory: projectUri,
135+
includeParentEnvironment: true,
136+
cCompilerConfig: await buildRunner.cCompilerConfig,
137+
buildResult: buildResult,
138+
// TODO(dcharkes): Fetch minimum MacOS version from somewhere. https://github.com/flutter/flutter/issues/145104
139+
targetMacOSVersion: 13,
140+
);
141+
ensureNativeAssetsLinkSucceed(linkResult);
142+
nativeAssets.addAll(linkResult.assets);
143+
dependencies.addAll(linkResult.dependencies);
144+
}
151145
}
152146
ensureNoLinkModeStatic(nativeAssets);
153147
globals.logger.printTrace('Building native assets for $targets done.');

‎packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ abstract class NativeAssetsBuildRunner {
6464
int? targetIOSVersion,
6565
int? targetMacOSVersion,
6666
IOSSdkImpl? targetIOSSdkImpl,
67+
required bool linkingEnabled,
6768
});
6869

6970
/// Runs all [packagesWithNativeAssets] `link.dart` in dry run.
@@ -169,6 +170,7 @@ class NativeAssetsBuildRunnerImpl implements NativeAssetsBuildRunner {
169170
targetOS: targetOS,
170171
workingDirectory: workingDirectory,
171172
packageLayout: packageLayout,
173+
linkingEnabled: false, // Dry run is only used in JIT mode.
172174
);
173175
}
174176

@@ -184,6 +186,7 @@ class NativeAssetsBuildRunnerImpl implements NativeAssetsBuildRunner {
184186
int? targetMacOSVersion,
185187
int? targetAndroidNdkApi,
186188
IOSSdkImpl? targetIOSSdkImpl,
189+
required bool linkingEnabled,
187190
}) {
188191
final PackageLayout packageLayout = PackageLayout.fromPackageConfig(
189192
packageConfig,
@@ -201,6 +204,7 @@ class NativeAssetsBuildRunnerImpl implements NativeAssetsBuildRunner {
201204
packageLayout: packageLayout,
202205
targetIOSVersion: targetIOSVersion,
203206
targetMacOSVersion: targetMacOSVersion,
207+
linkingEnabled: linkingEnabled,
204208
);
205209
}
206210

@@ -657,18 +661,8 @@ Future<Iterable<KernelAsset>> dryRunNativeAssetsSingleArchitectureInternal(
657661
includeParentEnvironment: true,
658662
);
659663
ensureNativeAssetsBuildDryRunSucceed(buildDryRunResult);
660-
final LinkDryRunResult linkDryRunResult = await buildRunner.linkDryRun(
661-
linkModePreference: LinkModePreferenceImpl.dynamic,
662-
targetOS: targetOS,
663-
workingDirectory: projectUri,
664-
includeParentEnvironment: true,
665-
buildDryRunResult: buildDryRunResult,
666-
);
667-
ensureNativeAssetsLinkDryRunSucceed(linkDryRunResult);
668-
final List<AssetImpl> nativeAssets = <AssetImpl>[
669-
...buildDryRunResult.assets,
670-
...linkDryRunResult.assets,
671-
];
664+
// No link hooks in JIT mode.
665+
final List<AssetImpl> nativeAssets = buildDryRunResult.assets;
672666
ensureNoLinkModeStatic(nativeAssets);
673667
globals.logger.printTrace('Dry running native assets for $targetOS done.');
674668
final Uri? absolutePath = flutterTester ? buildUri : null;
@@ -714,6 +708,7 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsSingleA
714708
}
715709

716710
final BuildModeImpl buildModeCli = nativeAssetsBuildMode(buildMode);
711+
final bool linkingEnabled = buildModeCli == BuildModeImpl.release;
717712

718713
globals.logger.printTrace('Building native assets for $target $buildModeCli.');
719714
final BuildResult buildResult = await buildRunner.build(
@@ -723,25 +718,29 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsSingleA
723718
workingDirectory: projectUri,
724719
includeParentEnvironment: true,
725720
cCompilerConfig: await buildRunner.cCompilerConfig,
721+
linkingEnabled: linkingEnabled,
726722
);
727723
ensureNativeAssetsBuildSucceed(buildResult);
728-
final LinkResult linkResult = await buildRunner.link(
729-
linkModePreference: LinkModePreferenceImpl.dynamic,
730-
target: target,
731-
buildMode: buildModeCli,
732-
workingDirectory: projectUri,
733-
includeParentEnvironment: true,
734-
cCompilerConfig: await buildRunner.cCompilerConfig,
735-
buildResult: buildResult,
736-
);
737-
ensureNativeAssetsLinkSucceed(linkResult);
724+
late final LinkResult linkResult;
725+
if (linkingEnabled) {
726+
linkResult = await buildRunner.link(
727+
linkModePreference: LinkModePreferenceImpl.dynamic,
728+
target: target,
729+
buildMode: buildModeCli,
730+
workingDirectory: projectUri,
731+
includeParentEnvironment: true,
732+
cCompilerConfig: await buildRunner.cCompilerConfig,
733+
buildResult: buildResult,
734+
);
735+
ensureNativeAssetsLinkSucceed(linkResult);
736+
}
738737
final List<AssetImpl> nativeAssets = <AssetImpl>[
739738
...buildResult.assets,
740-
...linkResult.assets,
739+
if (linkingEnabled) ...linkResult.assets,
741740
];
742741
final Set<Uri> dependencies = <Uri>{
743742
...buildResult.dependencies,
744-
...linkResult.dependencies,
743+
if (linkingEnabled) ...linkResult.dependencies,
745744
};
746745
ensureNoLinkModeStatic(nativeAssets);
747746
globals.logger.printTrace('Building native assets for $target done.');

‎packages/flutter_tools/pubspec.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ dependencies:
5656

5757
cli_config: 0.2.0
5858
graphs: 2.3.1
59-
native_assets_builder: 0.7.1
60-
native_assets_cli: 0.6.1
59+
native_assets_builder: 0.8.0
60+
native_assets_cli: 0.7.0
6161

6262
# We depend on very specific internal implementation details of the
6363
# 'test' package, which change between versions, so when upgrading
@@ -120,4 +120,4 @@ dartdoc:
120120
# Exclude this package from the hosted API docs.
121121
nodoc: true
122122

123-
# PUBSPEC CHECKSUM: 52ae
123+
# PUBSPEC CHECKSUM: 5aae

‎packages/flutter_tools/templates/package_ffi/pubspec.yaml.tmpl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ environment:
99
dependencies:
1010
cli_config: ^0.2.0
1111
logging: ^1.2.0
12-
native_assets_cli: ^0.6.1
13-
native_toolchain_c: ^0.5.0
12+
native_assets_cli: ^0.7.0
13+
native_toolchain_c: ^0.5.1
1414

1515
dev_dependencies:
1616
ffi: ^2.1.2

0 commit comments

Comments
 (0)