Skip to content

Commit 6e0edca

Browse files
authored
[hooks_runner] Optimize the no hooks path (#2302)
Bug: #2236 Optimize the code paths for when there are no hooks (speeds up standalone Dart). * Return early in various places, and check if there are hooks at all as first thing. * Write JSON encoding instead of YAML encoding. 17 ms -> 1 ms on a small JSON object. (And JSON is valid YAML.) After this the longest running part in `dartdev` is loading the `package_config.json`: 10 ms. We can't really avoid this because we need that info to check whether we have any hooks that need running. And the `package_config.json` isn't already loaded by dartdev (in contrast to `flutter_tools` where we have a `PackageConfig` object available). dartdev delegates compilation to `package:pub`(!) via `getExecutableForCommand` and that API uses the package config file path. dart-lang/pub#4067
1 parent 7bbea9c commit 6e0edca

File tree

6 files changed

+63
-53
lines changed

6 files changed

+63
-53
lines changed

pkgs/hooks_runner/lib/src/build_runner/build_planner.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ class NativeAssetsBuildPlanner {
116116
Future<Result<BuildPlan, HooksRunnerFailure>> makeBuildHookPlan() async {
117117
if (_buildHookPlan != null) return Success(_buildHookPlan!);
118118
final packagesWithNativeAssets = await packagesWithHook(Hook.build);
119+
if (packagesWithNativeAssets.isEmpty) {
120+
// Avoid calculating the package graph if there are no hooks.
121+
return const Success([]);
122+
}
119123
final packageMap = {
120124
for (final package in packagesWithNativeAssets) package.name: package,
121125
};

pkgs/hooks_runner/lib/src/build_runner/build_runner.dart

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,23 @@ class NativeAssetsBuildRunner {
113113
required List<ProtocolExtension> extensions,
114114
required bool linkingEnabled,
115115
}) async {
116+
final planResult = await _makePlan(hook: Hook.build, buildResult: null);
117+
if (planResult.isFailure) {
118+
return planResult.asFailure;
119+
}
120+
final (buildPlan, packageGraph) = planResult.success;
121+
if (buildPlan.isEmpty) {
122+
// Return eagerly if there are no build hooks at all.
123+
return Success(HookResult());
124+
}
125+
116126
final loadedUserDefines = await _loadedUserDefines;
117127
final hookResultUserDefines = await _checkUserDefines(loadedUserDefines);
118128
if (hookResultUserDefines.isFailure) {
119129
return hookResultUserDefines;
120130
}
121131
var hookResult = hookResultUserDefines.success;
122132

123-
final planResult = await _makePlan(hook: Hook.build, buildResult: null);
124-
if (planResult.isFailure) {
125-
return planResult.asFailure;
126-
}
127-
final (buildPlan, packageGraph) = planResult.success;
128-
129133
/// Key is packageName.
130134
final globalAssetsForBuild = <String, List<EncodedAsset>>{};
131135
for (final package in buildPlan) {

pkgs/hooks_runner/lib/src/model/kernel_assets.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
/// Dart SDK.
1515
library;
1616

17-
import '../utils/yaml.dart';
17+
import 'dart:convert';
18+
1819
import 'target.dart';
1920

2021
class KernelAssets {
@@ -30,7 +31,7 @@ class KernelAssets {
3031
assetsPerTarget[asset.target] = assets;
3132
}
3233

33-
final yamlContents = {
34+
final jsonContents = {
3435
'format-version': [1, 0, 0],
3536
'native-assets': {
3637
for (final entry in assetsPerTarget.entries)
@@ -40,7 +41,7 @@ class KernelAssets {
4041
},
4142
};
4243

43-
return yamlEncode(yamlContents);
44+
return const JsonEncoder.withIndent(' ').convert(jsonContents);
4445
}
4546
}
4647

pkgs/hooks_runner/lib/src/utils/yaml.dart

Lines changed: 0 additions & 15 deletions
This file was deleted.

pkgs/hooks_runner/pubspec.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ dependencies:
2323
package_config: ^2.1.0
2424
pub_semver: ^2.2.0
2525
yaml: ^3.1.3
26-
yaml_edit: ^2.2.2
2726

2827
dev_dependencies:
2928
custom_lint: ^0.7.5

pkgs/hooks_runner/test/model/kernel_assets_test.dart

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -52,37 +52,54 @@ void main() {
5252
),
5353
]);
5454

55-
final assetsDartEncoding = '''format-version:
56-
- 1
57-
- 0
58-
- 0
59-
native-assets:
60-
android_x64:
61-
foo:
62-
- absolute
63-
- ${fooUri.toFilePath()}
64-
foo2:
65-
- relative
66-
- ${foo2Uri.toFilePath()}
67-
foo3:
68-
- system
69-
- ${foo3Uri.toFilePath()}
70-
foo4:
71-
- executable
72-
foo5:
73-
- process
74-
linux_arm64:
75-
bar:
76-
- absolute
77-
- ${barUri.toFilePath()}
78-
windows_x64:
79-
bla:
80-
- absolute
81-
- ${blaUri.toFilePath()}''';
55+
final assetsDartEncoding = '''{
56+
"format-version": [
57+
1,
58+
0,
59+
0
60+
],
61+
"native-assets": {
62+
"android_x64": {
63+
"foo": [
64+
"absolute",
65+
"${fooUri.toFilePath()}"
66+
],
67+
"foo2": [
68+
"relative",
69+
"${foo2Uri.toFilePath()}"
70+
],
71+
"foo3": [
72+
"system",
73+
"${foo3Uri.toFilePath()}"
74+
],
75+
"foo4": [
76+
"executable"
77+
],
78+
"foo5": [
79+
"process"
80+
]
81+
},
82+
"linux_arm64": {
83+
"bar": [
84+
"absolute",
85+
"${barUri.toFilePath()}"
86+
]
87+
},
88+
"windows_x64": {
89+
"bla": [
90+
"absolute",
91+
"${blaUri.toFilePath()}"
92+
]
93+
}
94+
}
95+
}''';
8296

8397
test('asset yaml', () async {
8498
final fileContents = assets.toNativeAssetsFile();
85-
expect(fileContents, assetsDartEncoding);
99+
expect(
100+
fileContents.replaceAll(r'\ ', ' ').replaceAll(r'\\', r'\'),
101+
assetsDartEncoding,
102+
);
86103
});
87104

88105
test('path equality', () {

0 commit comments

Comments
 (0)