Skip to content

Commit 081b195

Browse files
authored
[native_assets_builder] Use file content hashing (#1750)
This PR changes the caching behavior for hooks to be file content hashing instead of last modified timestamps. Closes: #1593. In addition to using file hashes, a timestamp is passed in to detect if files were modified during a build. The moment of hashing contents is after a build is finished, but we should consider files changed after the build started to invalidate the build. If this happens, the build succeeds, but the cache is invalidated and a warning is printed to the logger. The implementation was modeled after the [`FileStore` in flutter_tools](https://github.com/flutter/flutter/blob/1e824af6bd217beb0cc201394c9832316b6150da/packages/flutter_tools/lib/src/build_system/file_store.dart). However, it was adapted to support caching directories and to match the code style in this repository. Directory caching is defined as follows: the hash of the names of the children. This excludes recursive descendants, and excludes the contents of children. For recursive caching (and glob patterns), the populator of the cache should do the glob/recursion to add all directories and files. ### Testing * The existing caching tests (such as `pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart`) cover caching behavior. * Now that last-modified are no longer used, some sleeps have been removed from tests. � ### Performance Adding a stopwatch to pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart for the second invocation of the hooks so that it is cached. * `lastModified` timestamps: 0.028 seconds (pre this PR) * `package:crypto` `md5`: 0.047 seconds (current PR) * `package:xxh3` `xxh3`: 0.042 seconds The implementation does not use parallel system IO for loading files (no `Future.wait`), but does use async I/O to allow flutter_tools to run other `Target`s in parallel. The (pre and post this PR) implementation is fast enough for a handful of packages with native assets in a `flutter run`. The implementation (pre and post this PR) is not fast enough for hot restart / hot reload with 10+ packages with native assets. So, when exploring support for that, we'll need to revisit the implementation. ### Related issues not addressed in this PR * #32 * Changing environment variables should also cause the hook to rerun. * #1578 * Hook compilation can potentially be shared. (This requires taking more directory locks due to concurrent and re-entrant invocations.)
1 parent bdb8a7d commit 081b195

File tree

7 files changed

+388
-65
lines changed

7 files changed

+388
-65
lines changed

pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

Lines changed: 68 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,14 @@ import 'package:logging/logging.dart';
1010
import 'package:native_assets_cli/native_assets_cli_internal.dart';
1111
import 'package:package_config/package_config.dart';
1212

13+
import '../dependencies_hash_file/dependencies_hash_file.dart';
1314
import '../locking/locking.dart';
1415
import '../model/build_dry_run_result.dart';
1516
import '../model/build_result.dart';
1617
import '../model/hook_result.dart';
1718
import '../model/link_result.dart';
1819
import '../package_layout/package_layout.dart';
19-
import '../utils/file.dart';
2020
import '../utils/run_process.dart';
21-
import '../utils/uri.dart';
2221
import 'build_planner.dart';
2322

2423
typedef DependencyMetadata = Map<String, Metadata>;
@@ -451,6 +450,8 @@ class NativeAssetsBuildRunner {
451450
return hookResult;
452451
}
453452

453+
// TODO(https://github.com/dart-lang/native/issues/32): Rerun hook if
454+
// environment variables change.
454455
Future<HookOutput?> _runHookForPackageCached(
455456
Hook hook,
456457
HookConfig config,
@@ -473,7 +474,7 @@ class NativeAssetsBuildRunner {
473474
final (
474475
compileSuccess,
475476
hookKernelFile,
476-
hookLastSourceChange,
477+
hookHashesFile,
477478
) = await _compileHookForPackageCached(
478479
config.packageName,
479480
config.outputDirectory,
@@ -488,7 +489,14 @@ class NativeAssetsBuildRunner {
488489

489490
final buildOutputFile =
490491
File.fromUri(config.outputDirectory.resolve(hook.outputName));
491-
if (buildOutputFile.existsSync()) {
492+
final dependenciesHashFile = File.fromUri(
493+
config.outputDirectory
494+
.resolve('../dependencies.dependencies_hash_file.json'),
495+
);
496+
final dependenciesHashes =
497+
DependenciesHashFile(file: dependenciesHashFile);
498+
final lastModifiedCutoffTime = DateTime.now();
499+
if (buildOutputFile.existsSync() && dependenciesHashFile.existsSync()) {
492500
late final HookOutput output;
493501
try {
494502
output = _readHookOutputFromUri(hook, buildOutputFile);
@@ -503,17 +511,13 @@ ${e.message}
503511
return null;
504512
}
505513

506-
final lastBuilt = output.timestamp.roundDownToSeconds();
507-
final dependenciesLastChange =
508-
await Dependencies(output.dependencies).lastModified();
509-
if (lastBuilt.isAfter(dependenciesLastChange) &&
510-
lastBuilt.isAfter(hookLastSourceChange)) {
514+
final outdated =
515+
(await dependenciesHashes.findOutdatedFileSystemEntity()) != null;
516+
if (!outdated) {
511517
logger.info(
512518
[
513519
'Skipping ${hook.name} for ${config.packageName} in $outDir.',
514-
'Last build on $lastBuilt.',
515-
'Last dependencies change on $dependenciesLastChange.',
516-
'Last hook change on $hookLastSourceChange.',
520+
'Last build on ${output.timestamp}.',
517521
].join(' '),
518522
);
519523
// All build flags go into [outDir]. Therefore we do not have to
@@ -522,7 +526,7 @@ ${e.message}
522526
}
523527
}
524528

525-
return await _runHookForPackage(
529+
final result = await _runHookForPackage(
526530
hook,
527531
config,
528532
validator,
@@ -533,6 +537,24 @@ ${e.message}
533537
hookKernelFile,
534538
packageLayout,
535539
);
540+
if (result == null) {
541+
if (await dependenciesHashFile.exists()) {
542+
await dependenciesHashFile.delete();
543+
}
544+
} else {
545+
final modifiedDuringBuild = await dependenciesHashes.hashFiles(
546+
[
547+
...result.dependencies,
548+
// Also depend on the hook source code.
549+
hookHashesFile.uri,
550+
],
551+
validBeforeLastModified: lastModifiedCutoffTime,
552+
);
553+
if (modifiedDuringBuild != null) {
554+
logger.severe('File modified during build. Build must be rerun.');
555+
}
556+
}
557+
return result;
536558
},
537559
);
538560
}
@@ -644,7 +666,10 @@ ${e.message}
644666
/// It does not reuse the cached kernel for different configs due to
645667
/// reentrancy requirements. For more info see:
646668
/// https://github.com/dart-lang/native/issues/1319
647-
Future<(bool success, File kernelFile, DateTime lastSourceChange)>
669+
///
670+
/// TODO(https://github.com/dart-lang/native/issues/1578): Compile only once
671+
/// instead of per config. This requires more locking.
672+
Future<(bool success, File kernelFile, File cacheFile)>
648673
_compileHookForPackageCached(
649674
String packageName,
650675
Uri outputDirectory,
@@ -659,29 +684,17 @@ ${e.message}
659684
final depFile = File.fromUri(
660685
outputDirectory.resolve('../hook.dill.d'),
661686
);
687+
final dependenciesHashFile = File.fromUri(
688+
outputDirectory.resolve('../hook.dependencies_hash_file.json'),
689+
);
690+
final dependenciesHashes = DependenciesHashFile(file: dependenciesHashFile);
691+
final lastModifiedCutoffTime = DateTime.now();
662692
final bool mustCompile;
663-
final DateTime sourceLastChange;
664-
if (!await depFile.exists()) {
693+
if (!await dependenciesHashFile.exists()) {
665694
mustCompile = true;
666-
sourceLastChange = DateTime.now();
667695
} else {
668-
// Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart`
669-
final depFileContents = await depFile.readAsString();
670-
final dartSourceFiles = depFileContents
671-
.trim()
672-
.split(' ')
673-
.skip(1) // '<kernel file>:'
674-
.map((u) => Uri.file(u).fileSystemEntity)
675-
.toList();
676-
final dartFilesLastChange = await dartSourceFiles.lastModified();
677-
final packageConfigLastChange =
678-
await packageConfigUri.fileSystemEntity.lastModified();
679-
sourceLastChange = packageConfigLastChange.isAfter(dartFilesLastChange)
680-
? packageConfigLastChange
681-
: dartFilesLastChange;
682-
final kernelLastChange = await kernelFile.lastModified();
683-
mustCompile = sourceLastChange == kernelLastChange ||
684-
sourceLastChange.isAfter(kernelLastChange);
696+
mustCompile =
697+
(await dependenciesHashes.findOutdatedFileSystemEntity()) != null;
685698
}
686699
final bool success;
687700
if (!mustCompile) {
@@ -696,8 +709,28 @@ ${e.message}
696709
kernelFile,
697710
depFile,
698711
);
712+
713+
if (success) {
714+
// Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart`
715+
final depFileContents = await depFile.readAsString();
716+
final dartSources = depFileContents
717+
.trim()
718+
.split(' ')
719+
.skip(1) // '<kernel file>:'
720+
.map(Uri.file)
721+
.toList();
722+
final modifiedDuringBuild = await dependenciesHashes.hashFiles(
723+
dartSources,
724+
validBeforeLastModified: lastModifiedCutoffTime,
725+
);
726+
if (modifiedDuringBuild != null) {
727+
logger.severe('File modified during build. Build must be rerun.');
728+
}
729+
} else {
730+
await dependenciesHashFile.delete();
731+
}
699732
}
700-
return (success, kernelFile, sourceLastChange);
733+
return (success, kernelFile, dependenciesHashFile);
701734
}
702735

703736
Future<bool> _compileHookForPackage(
@@ -859,12 +892,6 @@ ${compileResult.stdout}
859892
}
860893
}
861894

862-
extension on DateTime {
863-
DateTime roundDownToSeconds() =>
864-
DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch -
865-
millisecondsSinceEpoch % const Duration(seconds: 1).inMilliseconds);
866-
}
867-
868895
extension on Uri {
869896
Uri get parent => File(toFilePath()).parent.uri;
870897
}

0 commit comments

Comments
 (0)