From 58b6279c6ed91c3229467bbad04b490be8a13f4d Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Tue, 26 Nov 2024 20:09:08 +0100 Subject: [PATCH] [native_assets_builder] Use XXH3 hashing --- .../lib/src/build_runner/build_runner.dart | 113 ++++++---- .../file_system_cache/file_system_cache.dart | 201 ++++++++++++++++++ pkgs/native_assets_builder/pubspec.yaml | 1 + .../build_runner_caching_test.dart | 12 -- .../test/build_runner/build_runner_test.dart | 5 - .../test/build_runner/link_caching_test.dart | 7 - .../file_system_cache_test.dart | 115 ++++++++++ 7 files changed, 389 insertions(+), 65 deletions(-) create mode 100644 pkgs/native_assets_builder/lib/src/file_system_cache/file_system_cache.dart create mode 100644 pkgs/native_assets_builder/test/file_system_cache/file_system_cache_test.dart diff --git a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart index 1eea17597..52d212511 100644 --- a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart +++ b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart @@ -10,15 +10,14 @@ import 'package:logging/logging.dart'; import 'package:native_assets_cli/native_assets_cli_internal.dart'; import 'package:package_config/package_config.dart'; +import '../file_system_cache/file_system_cache.dart'; import '../locking/locking.dart'; import '../model/build_dry_run_result.dart'; import '../model/build_result.dart'; import '../model/hook_result.dart'; import '../model/link_result.dart'; import '../package_layout/package_layout.dart'; -import '../utils/file.dart'; import '../utils/run_process.dart'; -import '../utils/uri.dart'; import 'build_planner.dart'; typedef DependencyMetadata = Map; @@ -451,6 +450,8 @@ class NativeAssetsBuildRunner { return hookResult; } + // TODO(https://github.com/dart-lang/native/issues/32): Rerun hook if + // environment variables change. Future _runHookForPackageCached( Hook hook, HookConfig config, @@ -473,7 +474,7 @@ class NativeAssetsBuildRunner { final ( compileSuccess, hookKernelFile, - hookLastSourceChange, + hookCacheFile, ) = await _compileHookForPackageCached( config.packageName, config.outputDirectory, @@ -488,7 +489,13 @@ class NativeAssetsBuildRunner { final buildOutputFile = File.fromUri(config.outputDirectory.resolve(hook.outputName)); - if (buildOutputFile.existsSync()) { + final cacheFile = File.fromUri( + config.outputDirectory + .resolve('../dependencies.file_system_cache.json'), + ); + final cache = FileSystemCache(cacheFile: cacheFile); + final cacheCutoffTime = DateTime.now(); + if (buildOutputFile.existsSync() && cacheFile.existsSync()) { late final HookOutput output; try { output = _readHookOutputFromUri(hook, buildOutputFile); @@ -503,17 +510,13 @@ ${e.message} return null; } - final lastBuilt = output.timestamp.roundDownToSeconds(); - final dependenciesLastChange = - await Dependencies(output.dependencies).lastModified(); - if (lastBuilt.isAfter(dependenciesLastChange) && - lastBuilt.isAfter(hookLastSourceChange)) { + await cache.readCacheFile(); + final changedFile = await cache.findOutdatedFileSystemEntity(); + if (changedFile == null) { logger.info( [ 'Skipping ${hook.name} for ${config.packageName} in $outDir.', - 'Last build on $lastBuilt.', - 'Last dependencies change on $dependenciesLastChange.', - 'Last hook change on $hookLastSourceChange.', + 'Last build on ${output.timestamp}.', ].join(' '), ); // All build flags go into [outDir]. Therefore we do not have to @@ -522,7 +525,7 @@ ${e.message} } } - return await _runHookForPackage( + final result = await _runHookForPackage( hook, config, validator, @@ -533,6 +536,26 @@ ${e.message} hookKernelFile, packageLayout, ); + if (result == null) { + if (await cacheFile.exists()) { + await cacheFile.delete(); + } + } else { + cache.reset(); + final modifiedDuringBuild = await cache.hashFiles( + [ + ...result.dependencies, + // Also depend on the hook source code. + hookCacheFile.uri, + ], + validBeforeLastModified: cacheCutoffTime, + ); + await cache.persist(); + if (modifiedDuringBuild != null) { + logger.severe('File modified during build. Build must be rerun.'); + } + } + return result; }, ); } @@ -644,7 +667,10 @@ ${e.message} /// It does not reuse the cached kernel for different configs due to /// reentrancy requirements. For more info see: /// https://github.com/dart-lang/native/issues/1319 - Future<(bool success, File kernelFile, DateTime lastSourceChange)> + /// + /// TODO(https://github.com/dart-lang/native/issues/1578): Compile only once + /// instead of per config. This requires more locking. + Future<(bool success, File kernelFile, File cacheFile)> _compileHookForPackageCached( String packageName, Uri outputDirectory, @@ -659,29 +685,18 @@ ${e.message} final depFile = File.fromUri( outputDirectory.resolve('../hook.dill.d'), ); + final cacheFile = File.fromUri( + outputDirectory.resolve('../hook.file_system_cache.json'), + ); + final cache = FileSystemCache(cacheFile: cacheFile); + final cacheCutoffTime = DateTime.now(); final bool mustCompile; - final DateTime sourceLastChange; - if (!await depFile.exists()) { + if (!await cacheFile.exists()) { mustCompile = true; - sourceLastChange = DateTime.now(); } else { - // Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart` - final depFileContents = await depFile.readAsString(); - final dartSourceFiles = depFileContents - .trim() - .split(' ') - .skip(1) // ':' - .map((u) => Uri.file(u).fileSystemEntity) - .toList(); - final dartFilesLastChange = await dartSourceFiles.lastModified(); - final packageConfigLastChange = - await packageConfigUri.fileSystemEntity.lastModified(); - sourceLastChange = packageConfigLastChange.isAfter(dartFilesLastChange) - ? packageConfigLastChange - : dartFilesLastChange; - final kernelLastChange = await kernelFile.lastModified(); - mustCompile = sourceLastChange == kernelLastChange || - sourceLastChange.isAfter(kernelLastChange); + await cache.readCacheFile(); + final changedFile = await cache.findOutdatedFileSystemEntity(); + mustCompile = changedFile != null; } final bool success; if (!mustCompile) { @@ -696,8 +711,30 @@ ${e.message} kernelFile, depFile, ); + + if (success) { + // Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart` + final depFileContents = await depFile.readAsString(); + final dartSources = depFileContents + .trim() + .split(' ') + .skip(1) // ':' + .map(Uri.file) + .toList(); + cache.reset(); + final modifiedDuringBuild = await cache.hashFiles( + dartSources, + validBeforeLastModified: cacheCutoffTime, + ); + await cache.persist(); + if (modifiedDuringBuild != null) { + logger.severe('File modified during build. Build must be rerun.'); + } + } else { + await cacheFile.delete(); + } } - return (success, kernelFile, sourceLastChange); + return (success, kernelFile, cacheFile); } Future _compileHookForPackage( @@ -859,12 +896,6 @@ ${compileResult.stdout} } } -extension on DateTime { - DateTime roundDownToSeconds() => - DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch - - millisecondsSinceEpoch % const Duration(seconds: 1).inMilliseconds); -} - extension on Uri { Uri get parent => File(toFilePath()).parent.uri; } diff --git a/pkgs/native_assets_builder/lib/src/file_system_cache/file_system_cache.dart b/pkgs/native_assets_builder/lib/src/file_system_cache/file_system_cache.dart new file mode 100644 index 000000000..351db402f --- /dev/null +++ b/pkgs/native_assets_builder/lib/src/file_system_cache/file_system_cache.dart @@ -0,0 +1,201 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:convert'; +import 'dart:io'; + +import 'package:xxh3/xxh3.dart'; + +import '../utils/file.dart'; +import '../utils/uri.dart'; + +class FileSystemCache { + FileSystemCache({ + required File cacheFile, + }) : _cacheFile = cacheFile; + + final File _cacheFile; + FileSystemHashes _hashes = FileSystemHashes(); + + Future readCacheFile() async { + if (!await _cacheFile.exists()) { + _hashes = FileSystemHashes(); + return; + } + final jsonObject = + (json.decode(utf8.decode(await _cacheFile.readAsBytes())) as Map) + .cast(); + _hashes = FileSystemHashes.fromJson(jsonObject); + } + + void reset() => _hashes = FileSystemHashes(); + + /// Populate the cache with entries from [fileSystemEntities]. + /// + /// If [validBeforeLastModified] is provided, any entities that were modified + /// after [validBeforeLastModified] will get a dummy hash so that they will + /// show up as outdated. If any such entity exists, its uri will be returned. + Future hashFiles( + List fileSystemEntities, { + DateTime? validBeforeLastModified, + }) async { + Uri? modifiedAfterTimeStamp; + for (final uri in fileSystemEntities) { + int hash; + if (validBeforeLastModified != null && + (await uri.fileSystemEntity.lastModified()) + .isAfter(validBeforeLastModified)) { + hash = _hashLastModifiedAfterCutoff; + modifiedAfterTimeStamp = uri; + } else { + if (_isDirectoryPath(uri.path)) { + hash = await _hashDirectory(uri); + } else { + hash = await _hashFile(uri); + } + } + _hashes.files.add(FilesystemEntityHash(uri, hash)); + } + return modifiedAfterTimeStamp; + } + + Future persist() => + _cacheFile.writeAsString(json.encode(_hashes.toJson())); + + /// Find an outdated file or directory. + /// + /// If all + Future findOutdatedFileSystemEntity() async { + for (final cachedHash in _hashes.files) { + final uri = cachedHash.path; + final cachedHashValue = cachedHash.hash; + final int hashValue; + if (_isDirectoryPath(uri.path)) { + hashValue = await _hashDirectory(uri); + } else { + hashValue = await _hashFile(uri); + } + if (cachedHashValue != hashValue) { + return uri; + } + } + return null; + } + + Future _hashFile(Uri uri) async { + final file = File.fromUri(uri); + if (!await file.exists()) { + return _hashNotExists; + } + return xxh3(await file.readAsBytes()); + } + + Future _hashDirectory(Uri uri) async { + final directory = Directory.fromUri(uri); + if (!await directory.exists()) { + return _hashNotExists; + } + final children = directory.listSync(followLinks: true, recursive: false); + final childrenNames = children.map((e) => _pathBaseName(e.path)).join(';'); + return xxh3(utf8.encode(childrenNames)); + } + + /// Predefined hash for files and directories that do not exist. + /// + /// There are two predefined hash values. The chance that a predefined hash + /// collides with a real hash is 2/2^64. + static const _hashNotExists = 0; + + /// Predefined hash for files and directories that were modified after the + /// time that the cache was created. + /// + /// There are two predefined hash values. The chance that a predefined hash + /// collides with a real hash is 2/2^64. + static const _hashLastModifiedAfterCutoff = 1; +} + +/// Storage format for file system entity hashes. +/// +/// [File] hashes are a hash of the file. +/// +/// [Directory] hashes are a hash of the names of the direct children. +class FileSystemHashes { + FileSystemHashes({ + this.version = 1, + List? files, + }) : files = files ?? []; + + factory FileSystemHashes.fromJson(Map json) { + final version = json[_versionKey] as int; + final rawCachedFiles = + (json[_entitiesKey] as List).cast>(); + final files = [ + for (final Map rawFile in rawCachedFiles) + FilesystemEntityHash._fromJson(rawFile), + ]; + return FileSystemHashes( + version: version, + files: files, + ); + } + + final int version; + final List files; + + static const _versionKey = 'version'; + static const _entitiesKey = 'entities'; + + Map toJson() => { + _versionKey: version, + _entitiesKey: [ + for (final FilesystemEntityHash file in files) file.toJson(), + ], + }; +} + +/// A stored file or directory hash and path. +/// +/// [File] hashes are a hash of the file. +/// +/// [Directory] hashes are a hash of the names of the direct children. +class FilesystemEntityHash { + FilesystemEntityHash( + this.path, + this.hash, + ); + + factory FilesystemEntityHash._fromJson(Map json) => + FilesystemEntityHash( + _fileSystemPathToUri(json[_pathKey] as String), + json[_hashKey] as int, + ); + + static const _pathKey = 'path'; + static const _hashKey = 'hash'; + + final Uri path; + + /// A 64 bit hash. + /// + /// Typically xxh3. + final int hash; + + Object toJson() => { + _pathKey: path.toFilePath(), + _hashKey: hash, + }; +} + +bool _isDirectoryPath(String path) => + path.endsWith(Platform.pathSeparator) || path.endsWith('/'); + +Uri _fileSystemPathToUri(String path) { + if (_isDirectoryPath(path)) { + return Uri.directory(path); + } + return Uri.file(path); +} + +String _pathBaseName(String path) => + path.split(Platform.pathSeparator).where((e) => e.isNotEmpty).last; diff --git a/pkgs/native_assets_builder/pubspec.yaml b/pkgs/native_assets_builder/pubspec.yaml index a70c6ceb0..46f8a2a69 100644 --- a/pkgs/native_assets_builder/pubspec.yaml +++ b/pkgs/native_assets_builder/pubspec.yaml @@ -17,6 +17,7 @@ dependencies: native_assets_cli: path: ../native_assets_cli/ package_config: ^2.1.0 + xxh3: ^1.1.0 yaml: ^3.1.2 yaml_edit: ^2.1.0 diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart index e83948421..0df4b2478 100644 --- a/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart @@ -21,10 +21,6 @@ void main() async { workingDirectory: packageUri, 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.delayed(const Duration(seconds: 1)); { final logMessages = []; @@ -95,10 +91,6 @@ void main() async { workingDirectory: packageUri, 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.delayed(const Duration(seconds: 1)); { final result = (await build( @@ -151,10 +143,6 @@ void main() async { 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.delayed(const Duration(seconds: 1)); final result = (await build( packageUri, diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart index 51a5d68f2..239542042 100644 --- a/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart @@ -24,11 +24,6 @@ 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.delayed(const Duration(seconds: 1)); - // Trigger a build, should invoke build for libraries with native assets. { final logMessages = []; diff --git a/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart b/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart index 36e02c2cd..ded8c8a2a 100644 --- a/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart @@ -23,10 +23,6 @@ void main() async { workingDirectory: packageUri, 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.delayed(const Duration(seconds: 1)); final logMessages = []; late BuildResult buildResult; @@ -107,9 +103,6 @@ void main() async { sourceUri: testDataUri.resolve('simple_link_change_asset/'), targetUri: packageUri, ); - // Make sure the first hook is at least one second after the last - // change, or caching will not work. - await Future.delayed(const Duration(seconds: 1)); await runBuild(); expect(buildResult, isNotNull); diff --git a/pkgs/native_assets_builder/test/file_system_cache/file_system_cache_test.dart b/pkgs/native_assets_builder/test/file_system_cache/file_system_cache_test.dart new file mode 100644 index 000000000..24b930080 --- /dev/null +++ b/pkgs/native_assets_builder/test/file_system_cache/file_system_cache_test.dart @@ -0,0 +1,115 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; + +import 'package:native_assets_builder/src/file_system_cache/file_system_cache.dart'; +import 'package:test/test.dart'; + +import '../helpers.dart'; + +void main() async { + test('json format', () async { + await inTempDir((tempUri) async { + final hashes = FileSystemHashes( + files: [ + FilesystemEntityHash( + tempUri.resolve('foo.dll'), + 1337, + ), + ], + ); + final hashes2 = FileSystemHashes.fromJson(hashes.toJson()); + expect(hashes.files.single.path, equals(hashes2.files.single.path)); + expect(hashes.files.single.hash, equals(hashes2.files.single.hash)); + }); + }); + + test('file system cache', () async { + await inTempDir((tempUri) async { + final tempFile = File.fromUri(tempUri.resolve('foo.txt')); + final tempSubDir = Directory.fromUri(tempUri.resolve('subdir/')); + final subFile = File.fromUri(tempSubDir.uri.resolve('bar.txt')); + + final cacheFile = File.fromUri(tempUri.resolve('cache.json')); + final cache = FileSystemCache(cacheFile: cacheFile); + + Future reset() async { + await tempFile.create(recursive: true); + await tempSubDir.create(recursive: true); + await subFile.create(recursive: true); + await tempFile.writeAsString('hello'); + await subFile.writeAsString('world'); + + cache.reset(); + await cache.hashFiles([ + tempFile.uri, + tempSubDir.uri, + ]); + await cache.persist(); + expect(await cache.findOutdatedFileSystemEntity(), isNull); + } + + await reset(); + + // Change file contents. + await tempFile.writeAsString('asdf'); + expect(await cache.findOutdatedFileSystemEntity(), tempFile.uri); + await reset(); + + // Delete file. + await tempFile.delete(); + expect(await cache.findOutdatedFileSystemEntity(), tempFile.uri); + await reset(); + + // Add file to tracked directory. + final subFile2 = File.fromUri(tempSubDir.uri.resolve('baz.txt')); + await subFile2.create(recursive: true); + await subFile2.writeAsString('hello'); + expect(await cache.findOutdatedFileSystemEntity(), tempSubDir.uri); + await reset(); + + // Delete file from tracked directory. + await subFile.delete(); + expect(await cache.findOutdatedFileSystemEntity(), tempSubDir.uri); + await reset(); + + // Delete tracked directory. + await tempSubDir.delete(recursive: true); + expect(await cache.findOutdatedFileSystemEntity(), tempSubDir.uri); + await reset(); + + // Add directory to tracked directory. + final subDir2 = Directory.fromUri(tempSubDir.uri.resolve('baz/')); + await subDir2.create(recursive: true); + expect(await cache.findOutdatedFileSystemEntity(), tempSubDir.uri); + await reset(); + + // Overwriting a file with identical contents. + await tempFile.writeAsString('something something'); + await tempFile.writeAsString('hello'); + expect(await cache.findOutdatedFileSystemEntity(), isNull); + await reset(); + + // If a file is modified after the valid timestamp, it should be marked + // as changed. + cache.reset(); + await cache.hashFiles( + [ + tempFile.uri, + ], + validBeforeLastModified: (await tempFile.lastModified()) + .subtract(const Duration(seconds: 1)), + ); + expect(await cache.findOutdatedFileSystemEntity(), tempFile.uri); + await reset(); + + // Read a cache from file. + final cacheFromFile = FileSystemCache(cacheFile: cacheFile); + await cacheFromFile.readCacheFile(); + expect(await cacheFromFile.findOutdatedFileSystemEntity(), isNull); + }); + }); +}