diff --git a/lib/src/dart.dart b/lib/src/dart.dart index 4e1c4d1b7..362e94430 100644 --- a/lib/src/dart.dart +++ b/lib/src/dart.dart @@ -96,10 +96,13 @@ class AnalyzerErrorGroup implements Exception { /// /// If the compilation succeeds it is saved to a kernel file at [outputPath]. /// -/// If compilation fails, the output is cached at [incrementalDillOutputPath]. +/// If compilation fails, the output is cached at "[outputOPath].incremental". /// -/// Whichever of [incrementalDillOutputPath] and [outputPath] already exists is -/// used to initialize the compiler run. +/// Whichever of "[outputOPath].incremental" and [outputPath] already exists is +/// used to initialize the compiler run. To avoid the potential for +/// race-conditions, it is first copied to a temporary location, and atomically +/// moved to either [outputPath] or "[outputOPath].incremental" depending on the +/// result of compilation. /// /// The [packageConfigPath] should point at the package config file to be used /// for `package:` uri resolution. @@ -114,31 +117,29 @@ class AnalyzerErrorGroup implements Exception { /// assets map. Future precompile({ required String executablePath, - required String incrementalDillPath, required String name, required String outputPath, required String packageConfigPath, List additionalSources = const [], String? nativeAssets, }) async { - ensureDir(p.dirname(outputPath)); - ensureDir(p.dirname(incrementalDillPath)); - const platformDill = 'lib/_internal/vm_platform_strong.dill'; final sdkRoot = p.relative(p.dirname(p.dirname(Platform.resolvedExecutable))); String? tempDir; FrontendServerClient? client; try { - tempDir = createTempDir(p.dirname(incrementalDillPath), 'tmp'); + ensureDir(p.dirname(outputPath)); + final incrementalDillPath = '$outputPath.incremental'; + tempDir = createTempDir(p.dirname(outputPath), 'tmp'); // To avoid potential races we copy the incremental data to a temporary file // for just this compilation. final temporaryIncrementalDill = p.join(tempDir, '${p.basename(incrementalDillPath)}.temp'); try { - if (fileExists(incrementalDillPath)) { - copyFile(incrementalDillPath, temporaryIncrementalDill); - } else if (fileExists(outputPath)) { + if (fileExists(outputPath)) { copyFile(outputPath, temporaryIncrementalDill); + } else if (fileExists(incrementalDillPath)) { + copyFile(incrementalDillPath, temporaryIncrementalDill); } } on FileSystemException { // Not able to copy existing file, compilation will start from scratch. @@ -172,11 +173,14 @@ Future precompile({ // By using rename we ensure atomicity. An external observer will either // see the old or the new snapshot. renameFile(temporaryIncrementalDill, outputPath); + // Any old incremental data is deleted in case we started from a file on + // [incrementalDillPath]. + deleteEntry(incrementalDillPath); } else { // By using rename we ensure atomicity. An external observer will either // see the old or the new snapshot. renameFile(temporaryIncrementalDill, incrementalDillPath); - // If compilation failed we don't want to leave an incorrect snapshot. + // If compilation failed, don't leave an incorrect snapshot. tryDeleteEntry(outputPath); throw ApplicationException( diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 8d6dd67c2..1bdb2217f 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -220,10 +220,6 @@ class Entrypoint { /// The path to the directory containing dependency executable snapshots. String get _snapshotPath => p.join(cachePath, 'bin'); - /// The path to the directory containing previous dill files for incremental - /// builds. - String get _incrementalDillsPath => p.join(cachePath, 'incremental'); - Entrypoint._( this.rootDir, this._lockFile, @@ -522,7 +518,6 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without await dart.precompile( executablePath: await resolveExecutable(executable), outputPath: pathOfExecutable(executable), - incrementalDillPath: incrementalDillPathOfExecutable(executable), packageConfigPath: packageConfigPath, name: '$package:${p.basenameWithoutExtension(executable.relativePath)}', additionalSources: additionalSources, @@ -553,20 +548,6 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without ); } - String incrementalDillPathOfExecutable(Executable executable) { - assert(p.isRelative(executable.relativePath)); - return isGlobal - ? p.join( - _incrementalDillsPath, - '${p.basename(executable.relativePath)}.incremental.dill', - ) - : p.join( - _incrementalDillsPath, - executable.package, - '${p.basename(executable.relativePath)}.incremental.dill', - ); - } - /// The absolute path of [executable] resolved relative to [this]. Future resolveExecutable(Executable executable) async { return p.join( diff --git a/pubspec.yaml b/pubspec.yaml index bb1a0bf38..068f11128 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -9,6 +9,7 @@ dependencies: analyzer: ^5.1.0 args: ^2.4.0 async: ^2.6.1 + checks: ^0.2.2 cli_util: ^0.4.0 collection: ^1.15.0 convert: ^3.0.2 diff --git a/test/precompilation_test.dart b/test/precompilation_test.dart new file mode 100644 index 000000000..a749e36af --- /dev/null +++ b/test/precompilation_test.dart @@ -0,0 +1,104 @@ +import 'dart:io'; + +import 'package:checks/checks.dart'; +import 'package:pub/src/dart.dart'; +import 'package:pub/src/exceptions.dart'; +import 'package:pub/src/log.dart'; +import 'package:test/test.dart'; + +import 'descriptor.dart'; + +String outputPath() => '$sandbox/output/snapshot'; +String incrementalDillPath() => '${outputPath()}.incremental'; + +// A quite big program is needed for the caching to be an actual advantage. +FileDescriptor foo = file('foo.dart', ''' +foo() { + ${List.generate(500000, (index) => 'print("$index");').join('\n')} +} + '''); + +FileDescriptor workingMain = file( + 'main.dart', + ''' +import 'foo.dart'; + +main() async { + foo(); +} +''', +); + +FileDescriptor brokenMain = file( + 'main.dart', + ''' +import 'foo.dart'; +yadda yadda +main() asyncc { + foo(); +} +''', +); + +Future timeCompilation( + String executable, { + bool fails = false, +}) async { + final s = Stopwatch()..start(); + verbosity = Verbosity.none; + Future compile() async { + await precompile( + executablePath: executable, + name: 'abc', + outputPath: outputPath(), + packageConfigPath: path('app/.dart_tool/package_config.json'), + ); + } + + if (fails) { + await check(compile()).throws(); + } else { + await compile(); + } + verbosity = Verbosity.normal; + return s.elapsed; +} + +void main() { + test('Precompilation is much faster second time and removes old artifacts', + () async { + await dir('app', [ + workingMain, + foo, + packageConfigFile([]), + ]).create(); + final first = await timeCompilation(path('app/main.dart')); + check( + because: 'Should not leave a stray directory.', + File(incrementalDillPath()).existsSync(), + ).isFalse(); + check(File(outputPath()).existsSync()).isTrue(); + final second = await timeCompilation(path('app/main.dart')); + check(first).isGreaterThan(second * 2); + await dir('app', [ + brokenMain, + foo, + packageConfigFile([]), + ]).create(); + final afterErrors = + await timeCompilation(path('app/main.dart'), fails: true); + check(File(incrementalDillPath()).existsSync()).isTrue(); + check(File(outputPath()).existsSync()).isFalse(); + check(first).isGreaterThan(afterErrors * 2); + await dir('app', [ + workingMain, + ]).create(); + final afterFix = await timeCompilation(path('app/main.dart')); + check( + because: 'Should not leave a stray directory.', + Directory('${outputPath()}.incremental').existsSync(), + ).isFalse(); + check(File(outputPath()).existsSync()).isTrue(); + check(first).isGreaterThan(afterFix * 2); + }); +} diff --git a/tool/test.dart b/tool/test.dart index d0ffe95a8..2eb8adc95 100755 --- a/tool/test.dart +++ b/tool/test.dart @@ -30,16 +30,16 @@ Future main(List args) async { }); final pubSnapshotFilename = path.absolute(path.join('.dart_tool', '_pub', 'pub.dart.snapshot.dart2')); - final pubSnapshotIncrementalFilename = '$pubSnapshotFilename.incremental'; try { - stderr.writeln('Building snapshot'); + final stopwatch = Stopwatch()..start(); + stderr.write('Building snapshot...'); await precompile( executablePath: path.join('bin', 'pub.dart'), outputPath: pubSnapshotFilename, - incrementalDillPath: pubSnapshotIncrementalFilename, name: 'bin/pub.dart', packageConfigPath: path.join('.dart_tool', 'package_config.json'), ); + stderr.writeln(' (${stopwatch.elapsed.inMilliseconds}ms)'); testProcess = await Process.start( Platform.resolvedExecutable, ['run', 'test', ...args], @@ -51,11 +51,6 @@ Future main(List args) async { stderr.writeln('Failed building snapshot: $e'); exitCode = 1; } finally { - try { - await File(pubSnapshotFilename).delete(); - await sub.cancel(); - } on Exception { - // snapshot didn't exist. - } + await sub.cancel(); } }