Skip to content

Commit 09a756c

Browse files
authored
[hooks_runner] Add TimelineEvents (#2327)
This PR adds timeline events to the `hooks_runner` package: * All `FileSystem` calls. (With a `TracingFileSystem`.) * All `Process` runs. * The main methods. This enables looking at the performance: <img width="990" alt="image" src="https://github.com/user-attachments/assets/6a5c78be-46b7-4a0a-bc9a-ada325a30193" /> (First build compiles the hook and runs the hook. Second build doesn't compile the hook and doesn't run the hook. Side note: Compiling Dart takes longer than running the hook (compiling C)!) Some quirks: * The current approach only works for non-concurrent events. (The stdout and stderr file logging is concurrent to the process calls and therefore has to be excluded.) If we ever start doing things concurrently, we'll need to refactor this. * There are a lot of possibilities to improve things when we have more than one hook in the system. Optimizing this only makes sense once we have more real world projects using hooks. * Tracking issues for improving the performance: #1288 #1578 The one improvement made in this PR is that we stop hashing the kernel file contents. Saving a hook invocation if the kernel file didn't change (~300ms) if the hook sources only had a whitespace change seems less valuable than saving the hashing of the hook kernel file on every invocation (~50ms). After this change, cached hook invocations take ~30ms per hook on the local test cases. Closes: #2236
1 parent 108ae18 commit 09a756c

File tree

7 files changed

+637
-376
lines changed

7 files changed

+637
-376
lines changed

pkgs/hooks_runner/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 0.20.2
2+
3+
* Add `dart:developer` `TimelineEvent`s to enable performance tracing for
4+
hook invocations.
5+
16
## 0.20.1
27

38
* Bump the SDK constraint to at least the one from `package:hooks` to fix

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

Lines changed: 61 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'dart:convert';
6+
import 'dart:developer';
67

78
import 'package:file/file.dart';
89
import 'package:graphs/graphs.dart' as graphs;
@@ -19,13 +20,16 @@ typedef BuildPlan = List<Package>;
1920

2021
@internal
2122
class NativeAssetsBuildPlanner {
23+
final TimelineTask task;
24+
2225
final PackageGraph packageGraph;
2326
final Uri dartExecutable;
2427
final Logger logger;
2528
final PackageLayout packageLayout;
2629
final FileSystem fileSystem;
2730

2831
NativeAssetsBuildPlanner._({
32+
required this.task,
2933
required this.packageGraph,
3034
required this.dartExecutable,
3135
required this.logger,
@@ -39,11 +43,12 @@ class NativeAssetsBuildPlanner {
3943
required Logger logger,
4044
required PackageLayout packageLayout,
4145
required FileSystem fileSystem,
46+
TimelineTask? task,
4247
}) async {
4348
final packageGraphJsonFile = fileSystem.file(
4449
packageConfigUri.resolve('package_graph.json'),
4550
);
46-
assert(packageGraphJsonFile.existsSync());
51+
assert(await packageGraphJsonFile.exists());
4752
final packageGraphJson = await packageGraphJsonFile.readAsString();
4853
final packageGraph = PackageGraph.fromPackageGraphJsonString(
4954
packageGraphJson,
@@ -57,17 +62,22 @@ class NativeAssetsBuildPlanner {
5762
logger: logger,
5863
packageLayout: packageLayout,
5964
fileSystem: fileSystem,
65+
task: task ?? TimelineTask(),
6066
);
6167
}
6268

6369
/// All packages in [PackageLayout.packageConfig] with native assets.
6470
///
6571
/// Whether a package has native assets is defined by whether it contains
6672
/// a `hook/build.dart` or `hook/link.dart`.
67-
Future<List<Package>> packagesWithHook(Hook hook) async => switch (hook) {
68-
Hook.build => _packagesWithBuildHook ??= await _runPackagesWithHook(hook),
69-
Hook.link => _packagesWithLinkHook ??= await _runPackagesWithHook(hook),
70-
};
73+
Future<List<Package>> packagesWithHook(Hook hook) async => _timeAsync(
74+
'BuildPlanner.packagesWithHook',
75+
arguments: {'hook': hook.toString()},
76+
() async => switch (hook) {
77+
Hook.build => _packagesWithBuildHook ??= await _runPackagesWithHook(hook),
78+
Hook.link => _packagesWithLinkHook ??= await _runPackagesWithHook(hook),
79+
},
80+
);
7181

7282
List<Package>? _packagesWithBuildHook;
7383
List<Package>? _packagesWithLinkHook;
@@ -113,38 +123,53 @@ class NativeAssetsBuildPlanner {
113123
/// a cyclic dependency is detected among packages with native asset build
114124
/// hooks, the [Result] is a [Failure] containing a
115125
/// [HooksRunnerFailure.projectConfig].
116-
Future<Result<BuildPlan, HooksRunnerFailure>> makeBuildHookPlan() async {
117-
if (_buildHookPlan != null) return Success(_buildHookPlan!);
118-
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-
}
123-
final packageMap = {
124-
for (final package in packagesWithNativeAssets) package.name: package,
125-
};
126-
final packagesToBuild = packageMap.keys.toSet();
127-
final stronglyConnectedComponents = packageGraph.computeStrongComponents();
128-
final result = <Package>[];
129-
for (final stronglyConnectedComponent in stronglyConnectedComponents) {
130-
final stronglyConnectedComponentWithNativeAssets = [
131-
for (final packageName in stronglyConnectedComponent)
132-
if (packagesToBuild.contains(packageName)) packageName,
133-
];
134-
if (stronglyConnectedComponentWithNativeAssets.length > 1) {
135-
logger.severe(
136-
'Cyclic dependency for native asset builds in the following '
137-
'packages: $stronglyConnectedComponentWithNativeAssets.',
138-
);
139-
return const Failure(HooksRunnerFailure.projectConfig);
140-
} else if (stronglyConnectedComponentWithNativeAssets.length == 1) {
141-
result.add(
142-
packageMap[stronglyConnectedComponentWithNativeAssets.single]!,
143-
);
144-
}
126+
Future<Result<BuildPlan, HooksRunnerFailure>> makeBuildHookPlan() async =>
127+
_timeAsync('BuildPlanner.makeBuildHookPlan', () async {
128+
if (_buildHookPlan != null) return Success(_buildHookPlan!);
129+
final packagesWithNativeAssets = await packagesWithHook(Hook.build);
130+
if (packagesWithNativeAssets.isEmpty) {
131+
// Avoid calculating the package graph if there are no hooks.
132+
return const Success([]);
133+
}
134+
final packageMap = {
135+
for (final package in packagesWithNativeAssets) package.name: package,
136+
};
137+
final packagesToBuild = packageMap.keys.toSet();
138+
final stronglyConnectedComponents = packageGraph
139+
.computeStrongComponents();
140+
final result = <Package>[];
141+
for (final stronglyConnectedComponent in stronglyConnectedComponents) {
142+
final stronglyConnectedComponentWithNativeAssets = [
143+
for (final packageName in stronglyConnectedComponent)
144+
if (packagesToBuild.contains(packageName)) packageName,
145+
];
146+
if (stronglyConnectedComponentWithNativeAssets.length > 1) {
147+
logger.severe(
148+
'Cyclic dependency for native asset builds in the following '
149+
'packages: $stronglyConnectedComponentWithNativeAssets.',
150+
);
151+
return const Failure(HooksRunnerFailure.projectConfig);
152+
} else if (stronglyConnectedComponentWithNativeAssets.length == 1) {
153+
result.add(
154+
packageMap[stronglyConnectedComponentWithNativeAssets.single]!,
155+
);
156+
}
157+
}
158+
_buildHookPlan = result;
159+
return Success(result);
160+
});
161+
162+
Future<T> _timeAsync<T>(
163+
String name,
164+
Future<T> Function() function, {
165+
Map<String, Object>? arguments,
166+
}) async {
167+
task.start(name, arguments: arguments);
168+
try {
169+
return await function();
170+
} finally {
171+
task.finish();
145172
}
146-
_buildHookPlan = result;
147-
return Success(result);
148173
}
149174
}
150175

0 commit comments

Comments
 (0)