Revert "[vm, front-end server] Cache generated bytecode using separate file"

This reverts commit 86641d0.

Reason for revert: Appears to break Flutter 3H:

- https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8897037674557388176/+/steps/flutter_test_framework_coverage/0/stdout

- https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8897037674557388176/+/steps/flutter_test_framework_tests/0/stdout

- https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8897037674557388176/+/steps/flutter_test_hostonly_devicelab_tests/0/stdout

Original change's description:
> [vm, front-end server] Cache generated bytecode using separate file
> 
> Previously, generated bytecode was cached using metadata in kernel AST.
> When serializaing kernel AST (for subsequent compilations) incremental
> serialization was not used, so AST serialization was considerably
> slower in bytecode mode compared to non-bytecode mode.
> 
> Apparently, metadata is not compatible with incremental serialization
> (dart-lang/sdk#39302).
> 
> This change reimplements caching of generated bytecode using a separate
> 'foo.dill.cache' file near the generated 'foo.dill' and 'foo.dill.ast'.
> AST is now written using incremental serialization both in bytecode
> and non-bytecode modes.
> 
> flutter_test_performance benchmark:
> 
> AST mode:
> 
>   "without_change_elapsed_time_ms": 2615-2652,
>   "implementation_change_elapsed_time_ms": 6292-6394,
>   "interface_change_elapsed_time_ms": 6183-6484,
>   "with_coverage_time_ms": 2723-2834
> 
> Bytecode mode, before this change:
> 
>   "without_change_elapsed_time_ms": 3246-3295,
>   "implementation_change_elapsed_time_ms": 7998-8068,
>   "interface_change_elapsed_time_ms": 7899-8029,
>   "with_coverage_time_ms": 3316-3378
> 
> Bytecode mode, after this change:
> 
>   "without_change_elapsed_time_ms": 2689-2737,
>   "implementation_change_elapsed_time_ms": 7630-7677,
>   "interface_change_elapsed_time_ms": 7724-7917,
>   "with_coverage_time_ms": 2841-2949
> 
> Change-Id: I603d33f49949146cf6620226d810f2f3cb9853a9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124592
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>

TBR=rmacnak@google.com,alexmarkov@google.com

Change-Id: Ib433b68b7de3b08179c9c080a10f62703cd2546d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124983
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>

Author: Clement Skau

pkg/frontend_server/lib/frontend_server.dart

@@ -28,7 +28,9 @@ import 'package:kernel/target/targets.dart' show targets, TargetFlags;
 import 'package:path/path.dart' as path;
 import 'package:usage/uuid/uuid.dart';
 
-import 'package:vm/binary_cache.dart' show BinaryCache, Range;
+import 'package:vm/metadata/binary_cache.dart'
+    show BinaryCacheMetadataRepository;
+
 import 'package:vm/bytecode/gen_bytecode.dart'
     show generateBytecode, createFreshComponentWithBytecode;
 import 'package:vm/bytecode/options.dart' show BytecodeOptions;
@@ -281,8 +283,6 @@ class FrontendCompiler implements CompilerInterface {
   String _kernelBinaryFilenameIncremental;
   String _kernelBinaryFilenameFull;
   String _initializeFromDill;
-  String _bytecodeDillPath;
-  String _bytecodeCachePath;
 
   Set<Uri> previouslyReportedDependencies = Set<Uri>();
 
@@ -404,14 +404,12 @@ class FrontendCompiler implements CompilerInterface {
       ];
     }
 
-    _bytecodeCachePath = _initializeFromDill + ".cache";
-    if (compilerOptions.bytecode) {
+    if (compilerOptions.bytecode && _initializeFromDill != null) {
       // If we are generating bytecode, put bytecode only (not AST) in
       // [_kernelBinaryFilename], which the user of this tool will eventually
       // feed to Flutter engine or flutter_tester. Use a separate file to cache
       // the AST result to initialize the incremental compiler for the next
       // invocation of this tool.
-      _bytecodeDillPath = _initializeFromDill;
       _initializeFromDill += ".ast";
     }
 
@@ -558,38 +556,25 @@ class FrontendCompiler implements CompilerInterface {
       {bool filterExternal: false,
       IncrementalSerializer incrementalSerializer}) async {
     final Component component = results.component;
+    // Remove the cache that came either from this function or from
+    // initializing from a kernel file.
+    component.metadata.remove(BinaryCacheMetadataRepository.repositoryTag);
 
     if (_compilerOptions.bytecode) {
-      BinaryCache inputCache;
-      if (_options['incremental']) {
-        inputCache = new BinaryCache();
-        await inputCache.read(_bytecodeCachePath, _bytecodeDillPath);
-      }
-      BinaryCache outputCache = new BinaryCache();
-
       {
         // Generate bytecode as the output proper.
         final IOSink sink = File(filename).openWrite();
-        int offset = 0;
         await runWithFrontEndCompilerContext(
             _mainSource, _compilerOptions, component, () async {
           if (_options['incremental']) {
             // When loading a single kernel buffer with multiple sub-components,
             // the VM expects 'main' to be the first sub-component.
             await forEachPackage(component,
                 (String package, List<Library> libraries) async {
-              offset = _writePackage(
-                  results,
-                  package,
-                  libraries,
-                  sink,
-                  offset,
-                  inputCache,
-                  outputCache,
-                  incrementalSerializer?.invalidatedUris);
+              _writePackage(results, package, libraries, sink);
             }, mainFirst: true);
           } else {
-            _writePackage(results, 'main', component.libraries, sink, 0);
+            _writePackage(results, 'main', component.libraries, sink);
           }
         });
         await sink.close();
@@ -600,6 +585,15 @@ class FrontendCompiler implements CompilerInterface {
         // of [filename] so that a later invocation of frontend_server will the
         // same arguments will use this to initialize its incremental kernel
         // compiler.
+        final repository = BinaryCacheMetadataRepository();
+        component.addMetadataRepository(repository);
+        for (var lib in component.libraries) {
+          var bytes = BinaryCacheMetadataRepository.lookup(lib);
+          if (bytes != null) {
+            repository.mapping[lib] = bytes;
+          }
+        }
+
         final file = new File(_initializeFromDill);
         await file.create(recursive: true);
         final IOSink sink = file.openWrite();
@@ -613,18 +607,9 @@ class FrontendCompiler implements CompilerInterface {
 
         sortComponent(component);
 
-        if (incrementalSerializer != null) {
-          incrementalSerializer.writePackagesToSinkAndTrimComponent(
-              component, sink);
-        } else if (unsafePackageSerialization == true) {
-          writePackagesToSinkAndTrimComponent(component, sink);
-        }
-
         printer.writeComponentFile(component);
         await sink.close();
       }
-
-      await outputCache.write(_bytecodeCachePath);
     } else {
       // Generate AST as the output proper.
       final IOSink sink = File(filename).openWrite();
@@ -647,9 +632,6 @@ class FrontendCompiler implements CompilerInterface {
 
       printer.writeComponentFile(component);
       await sink.close();
-
-      // Reset cache if not generating bytecode.
-      await new File(_bytecodeCachePath).writeAsString('');
     }
 
     if (_options['split-output-by-packages']) {
@@ -732,33 +714,18 @@ class FrontendCompiler implements CompilerInterface {
     }
   }
 
-  int _writePackage(KernelCompilationResults result, String package,
-      List<Library> libraries, IOSink sink, int offset,
-      [BinaryCache inputCache,
-      BinaryCache outputCache,
-      Set<Uri> invalidatedUris]) {
+  void _writePackage(KernelCompilationResults result, String package,
+      List<Library> libraries, IOSink sink) {
     final canCache = libraries.isNotEmpty &&
         _compilerOptions.bytecode &&
         errors.isEmpty &&
         package != "main";
 
-    if (canCache && invalidatedUris != null) {
-      final range = inputCache?.get(package);
-      if (range != null) {
-        bool invalidated = false;
-        for (var lib in libraries) {
-          if (invalidatedUris.contains(lib.fileUri)) {
-            invalidated = true;
-            break;
-          }
-        }
-
-        if (!invalidated) {
-          final cachedBytes = inputCache.getBytes(range);
-          sink.add(cachedBytes);
-          outputCache.add(package, Range(offset, cachedBytes.length));
-          return offset + cachedBytes.length;
-        }
+    if (canCache) {
+      var cachedBytes = BinaryCacheMetadataRepository.lookup(libraries.first);
+      if (cachedBytes != null) {
+        sink.add(cachedBytes);
+        return;
       }
     }
 
@@ -783,9 +750,8 @@ class FrontendCompiler implements CompilerInterface {
     final bytes = byteSink.builder.takeBytes();
     sink.add(bytes);
     if (canCache) {
-      outputCache.add(package, Range(offset, bytes.length));
+      BinaryCacheMetadataRepository.insert(libraries.first, bytes);
     }
-    return offset + bytes.length;
   }
 
   @override

pkg/vm/lib/binary_cache.dart

@@ -0,0 +1,37 @@
+// Copyright (c) 2019, 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.
+
+library vm.metadata.binary_cache;
+
+import 'package:kernel/ast.dart'
+    show BinarySink, BinarySource, MetadataRepository, Node, TreeNode;
+
+class BinaryCacheMetadataRepository extends MetadataRepository<List<int>> {
+  static const repositoryTag = 'vm.binary_cache';
+
+  @override
+  String get tag => repositoryTag;
+
+  @override
+  final Map<TreeNode, List<int>> mapping = <TreeNode, List<int>>{};
+
+  @override
+  void writeToBinary(List<int> metadata, Node node, BinarySink sink) {
+    sink.writeByteList(metadata);
+  }
+
+  @override
+  List<int> readFromBinary(Node node, BinarySource source) {
+    List<int> result = source.readByteList();
+    _weakMap[node] = result;
+    return result;
+  }
+
+  static List<int> lookup(Node node) => _weakMap[node];
+  static void insert(Node node, List<int> metadata) {
+    _weakMap[node] = metadata;
+  }
+
+  static final _weakMap = new Expando<List<int>>();
+}

pkg/vm/lib/metadata/binary_cache.dart

@@ -17,6 +17,7 @@ import 'package:kernel/transformations/continuation.dart' as transformAsync
 import 'package:kernel/vm/constants_native_effects.dart'
     show VmConstantsBackend;
 
+import '../metadata/binary_cache.dart' show BinaryCacheMetadataRepository;
 import '../transformations/call_site_annotator.dart' as callSiteAnnotator;
 import '../transformations/list_factory_specializer.dart'
     as listFactorySpecializer;
@@ -369,6 +370,7 @@ class VmTarget extends Target {
   @override
   Component configureComponent(Component component) {
     callSiteAnnotator.addRepositoryTo(component);
+    component.addMetadataRepository(new BinaryCacheMetadataRepository());
     return super.configureComponent(component);
   }