Skip to content

Commit

Permalink
[native_assets_builder] Fix hook kernel compilation caching (#1406)
Browse files Browse the repository at this point in the history
  • Loading branch information
dcharkes authored Aug 9, 2024
1 parent 2464aaa commit 020fe54
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
22 changes: 17 additions & 5 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -610,9 +610,20 @@ ${e.message}
}
}

/// Compiles the hook to dill and caches the dill.
/// Compiles the hook to kernel and caches the kernel.
///
/// It does not reuse the cached dill for different [config]s, due to
/// If any of the Dart source files, or the package config changed after
/// the last time the kernel file is compiled, the kernel file is
/// recompiled. Otherwise a cached version is used.
///
/// Due to some OSes only providing last-modified timestamps with second
/// precision. The kernel compilation cache might be considered stale if
/// the last modification and the kernel compilation happened within one
/// second of each other. We error on the side of caution, rather recompile
/// one time too many, then not recompiling when recompilation should have
/// happened.
///
/// It does not reuse the cached kernel for different [config]s, due to
/// reentrancy requirements. For more info see:
/// https://github.com/dart-lang/native/issues/1319
Future<(bool success, File kernelFile, DateTime lastSourceChange)>
Expand All @@ -639,7 +650,7 @@ ${e.message}
final dartSourceFiles = depFileContents
.trim()
.split(' ')
.skip(1) // '<dill>:'
.skip(1) // '<kernel file>:'
.map((u) => Uri.file(u).fileSystemEntity)
.toList();
final dartFilesLastChange = await dartSourceFiles.lastModified();
Expand All @@ -648,8 +659,9 @@ ${e.message}
sourceLastChange = packageConfigLastChange.isAfter(dartFilesLastChange)
? packageConfigLastChange
: dartFilesLastChange;
final dillLastChange = await kernelFile.lastModified();
mustCompile = sourceLastChange.isAfter(dillLastChange);
final kernelLastChange = await kernelFile.lastModified();
mustCompile = sourceLastChange == kernelLastChange ||
sourceLastChange.isAfter(kernelLastChange);
}
final bool success;
if (!mustCompile) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,36 +110,55 @@ void main() async {
test(
'add C file, modify hook',
timeout: longTimeout,
onPlatform: {
'linux': const Skip('https://github.com/dart-lang/native/issues/1391.'),
},
() async {
await inTempDir((tempUri) async {
await copyTestProjects(targetUri: tempUri);
final packageUri = tempUri.resolve('native_add/');

final logMessages = <String>[];
final logger = createCapturingLogger(logMessages);

await runPubGet(workingDirectory: packageUri, logger: logger);
logMessages.clear();
// Make sure the first compile is at least one second after the
// package_config.json is written, otherwise dill compilation isn't
// cached.
await Future<void>.delayed(const Duration(seconds: 1));

final result = await build(packageUri, logger, dartExecutable);
{
final result = await build(packageUri, logger, dartExecutable);
await expectSymbols(
asset: result.assets.single as NativeCodeAssetImpl,
symbols: ['add']);
final compiledHook = logMessages
.where((m) =>
m.contains('dart compile kernel') ||
m.contains('dart.exe compile kernel'))
.isNotEmpty;
expect(compiledHook, isTrue);
}
logMessages.clear();
await expectSymbols(
asset: result.assets.single as NativeCodeAssetImpl,
symbols: ['add'],
);

await copyTestProjects(
sourceUri: testDataUri.resolve('native_add_add_source/'),
targetUri: packageUri);

{
final result = await build(packageUri, logger, dartExecutable);
{
final compiledHook = logMessages
.where((m) =>
m.contains('dart compile kernel') ||
m.contains('dart.exe compile kernel'))
.isNotEmpty;
expect(compiledHook, isTrue);
}
logMessages.clear();
await expectSymbols(
asset: result.assets.single as NativeCodeAssetImpl,
symbols: ['add', 'multiply']);
asset: result.assets.single as NativeCodeAssetImpl,
symbols: ['add', 'multiply'],
);
}
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ void main() async {
logger: logger,
);

// Make sure the first compile is at least one second after the
// package_config.json is written, otherwise dill compilation isn't
// cached.
await Future<void>.delayed(const Duration(seconds: 1));

// Trigger a build, should invoke build for libraries with native assets.
{
final logMessages = <String>[];
Expand Down

0 comments on commit 020fe54

Please sign in to comment.