[vm] Test compilation traces, type feedback, reused instructions.

Improve error for failure to reuse instructions.

Change-Id: I38b30a0da508f686d73dffebbf5893d6c939f19a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95940
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

runtime/bin/main.cc

@@ -883,6 +883,13 @@ bool RunMainIsolate(const char* script_name, CommandLineOptions* dart_options) {
       LoadBytecode();
     }
 
+    if (Options::load_compilation_trace_filename() != NULL) {
+      uint8_t* buffer = NULL;
+      intptr_t size = 0;
+      ReadFile(Options::load_compilation_trace_filename(), &buffer, &size);
+      result = Dart_LoadCompilationTrace(buffer, size);
+      CHECK_RESULT(result);
+    }
     if (Options::load_type_feedback_filename() != NULL) {
       uint8_t* buffer = NULL;
       intptr_t size = 0;
@@ -926,6 +933,13 @@ bool RunMainIsolate(const char* script_name, CommandLineOptions* dart_options) {
     }
     CHECK_RESULT(result);
 
+    if (Options::save_compilation_trace_filename() != NULL) {
+      uint8_t* buffer = NULL;
+      intptr_t size = 0;
+      result = Dart_SaveCompilationTrace(&buffer, &size);
+      CHECK_RESULT(result);
+      WriteFile(Options::save_compilation_trace_filename(), buffer, size);
+    }
     if (Options::save_type_feedback_filename() != NULL) {
       uint8_t* buffer = NULL;
       intptr_t size = 0;

runtime/bin/main_options.h

@@ -24,6 +24,8 @@ namespace bin {
   V(depfile, depfile)                                                          \
   V(depfile_output_filename, depfile_output_filename)                          \
   V(shared_blobs, shared_blobs_filename)                                       \
+  V(save_compilation_trace, save_compilation_trace_filename)                   \
+  V(load_compilation_trace, load_compilation_trace_filename)                   \
   V(save_type_feedback, save_type_feedback_filename)                           \
   V(load_type_feedback, load_type_feedback_filename)                           \
   V(root_certs_file, root_certs_file)                                          \

runtime/tests/vm/dart/compilation_trace_test.dart

@@ -0,0 +1,45 @@
+// 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.
+
+import "dart:async";
+import "dart:io";
+
+import "package:path/path.dart" as p;
+
+import "snapshot_test_helper.dart";
+
+int fib(int n) {
+  if (n <= 1) return 1;
+  return fib(n - 1) + fib(n - 2);
+}
+
+Future<void> main(List<String> args) async {
+  if (args.contains("--child")) {
+    print(fib(35));
+    return;
+  }
+
+  if (!Platform.script.toString().endsWith(".dart")) {
+    print("This test must run from source");
+    return;
+  }
+
+  await withTempDir((String tmp) async {
+    final String tracePath = p.join(tmp, "compilation_trace.txt");
+
+    final result1 = await runDart("generate compilation trace", [
+      "--save_compilation_trace=$tracePath",
+      Platform.script.toFilePath(),
+      "--child",
+    ]);
+    expectOutput("14930352", result1);
+
+    final result2 = await runDart("use compilation trace", [
+      "--load_compilation_trace=$tracePath",
+      Platform.script.toFilePath(),
+      "--child",
+    ]);
+    expectOutput("14930352", result2);
+  });
+}

runtime/tests/vm/dart/reused_instructions_test.dart

@@ -0,0 +1,92 @@
+// 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.
+
+import "dart:async";
+import "dart:io";
+
+import "package:path/path.dart" as p;
+
+import "snapshot_test_helper.dart";
+
+int fib(int n) {
+  if (n <= 1) return 1;
+  return fib(n - 1) + fib(n - 2);
+}
+
+Future<void> main(List<String> args) async {
+  if (args.contains("--child")) {
+    print(fib(35));
+    return;
+  }
+
+  if (!Platform.script.toString().endsWith(".dart")) {
+    print("This test must run from source");
+    return;
+  }
+
+  await withTempDir((String tmp) async {
+    final String coreVMDataPath = p.join(tmp, "core_vm_snapshot_data.bin");
+    final String coreIsoDataPath =
+        p.join(tmp, "core_isolate_snapshot_data.bin");
+    final String baselineIsoDataPath =
+        p.join(tmp, "baseline_isolate_snapshot_data.bin");
+    final String baselineIsoInstrPath =
+        p.join(tmp, "baseline_isolate_snapshot_instructions.bin");
+    final String patchIsoDataPath =
+        p.join(tmp, "patch_isolate_snapshot_data.bin");
+    final String patchIsoInstrPath =
+        p.join(tmp, "patch_isolate_snapshot_instr.bin");
+    final String kernelPath = p.join(tmp, "app.dill");
+    final String tracePath = p.join(tmp, "compilation_trace.txt");
+
+    // We don't support snapshot with code on IA32.
+    final String appSnapshotKind =
+        Platform.version.contains("ia32") ? "app" : "app-jit";
+
+    final result1 = await runGenKernel("generate kernel", [
+      Platform.script.toFilePath(),
+      "--output",
+      kernelPath,
+    ]);
+    expectOutput("", result1);
+
+    final result2 = await runDart("generate compilation trace", [
+      "--save_compilation_trace=$tracePath",
+      kernelPath,
+      "--child",
+    ]);
+    expectOutput("14930352", result2);
+
+    final result3 = await runGenSnapshot("generate core snapshot", [
+      "--snapshot_kind=core",
+      "--vm_snapshot_data=${coreVMDataPath}",
+      "--isolate_snapshot_data=${coreIsoDataPath}",
+      platformDill,
+    ]);
+    expectOutput("", result3);
+
+    final result4 = await runGenSnapshot("generate baseline app snapshot", [
+      "--snapshot_kind=${appSnapshotKind}",
+      "--load_vm_snapshot_data=${coreVMDataPath}",
+      "--load_isolate_snapshot_data=${coreIsoDataPath}",
+      "--isolate_snapshot_data=${baselineIsoDataPath}",
+      "--isolate_snapshot_instructions=${baselineIsoInstrPath}",
+      "--load_compilation_trace=$tracePath",
+      kernelPath,
+    ]);
+    expectOutput("", result4);
+
+    final result5 = await runGenSnapshot("generate patch app snapshot", [
+      "--snapshot_kind=${appSnapshotKind}",
+      "--load_vm_snapshot_data=${coreVMDataPath}",
+      "--load_isolate_snapshot_data=${coreIsoDataPath}",
+      "--isolate_snapshot_data=${patchIsoDataPath}",
+      "--isolate_snapshot_instructions=${patchIsoInstrPath}",
+      "--reused_instructions=${baselineIsoInstrPath}",
+      "--load_compilation_trace=$tracePath",
+      kernelPath,
+    ]);
+    expectOutput("", result5);
+  });
+}

runtime/tests/vm/dart/shared_snapshot_test.dart

@@ -0,0 +1,93 @@
+// 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.
+
+import "dart:async";
+import "dart:io";
+
+import "package:path/path.dart" as p;
+
+import "snapshot_test_helper.dart";
+
+int fib(int n) {
+  if (n <= 1) return 1;
+  return fib(n - 1) + fib(n - 2);
+}
+
+Future<void> main(List<String> args) async {
+  if (args.contains("--child")) {
+    print(fib(35));
+    return;
+  }
+
+  if (!Platform.script.toString().endsWith(".dart")) {
+    print("This test must run from source");
+    return;
+  }
+
+  await withTempDir((String tmp) async {
+    final String coreVMDataPath = p.join(tmp, "core_vm_snapshot_data.bin");
+    final String coreIsoDataPath =
+        p.join(tmp, "core_isolate_snapshot_data.bin");
+    final String baselineIsoDataPath =
+        p.join(tmp, "baseline_isolate_snapshot_data.bin");
+    final String baselineIsoInstrPath =
+        p.join(tmp, "baseline_isolate_snapshot_instructions.bin");
+    final String patchIsoDataPath =
+        p.join(tmp, "patch_isolate_snapshot_data.bin");
+    final String patchIsoInstrPath =
+        p.join(tmp, "patch_isolate_snapshot_instr.bin");
+    final String kernelPath = p.join(tmp, "app.dill");
+    final String tracePath = p.join(tmp, "compilation_trace.txt");
+
+    // We don't support snapshot with code on IA32.
+    final String appSnapshotKind =
+        Platform.version.contains("ia32") ? "app" : "app-jit";
+
+    final result1 = await runGenKernel("generate kernel", [
+      Platform.script.toFilePath(),
+      "--output",
+      kernelPath,
+    ]);
+    expectOutput("", result1);
+
+    final result2 = await runDart("generate compilation trace", [
+      "--save_compilation_trace=$tracePath",
+      kernelPath,
+      "--child",
+    ]);
+    expectOutput("14930352", result2);
+
+    final result3 = await runGenSnapshot("generate core snapshot", [
+      "--snapshot_kind=core",
+      "--vm_snapshot_data=${coreVMDataPath}",
+      "--isolate_snapshot_data=${coreIsoDataPath}",
+      platformDill,
+    ]);
+    expectOutput("", result3);
+
+    final result4 = await runGenSnapshot("generate baseline app snapshot", [
+      "--snapshot_kind=${appSnapshotKind}",
+      "--load_vm_snapshot_data=${coreVMDataPath}",
+      "--load_isolate_snapshot_data=${coreIsoDataPath}",
+      "--isolate_snapshot_data=${baselineIsoDataPath}",
+      "--isolate_snapshot_instructions=${baselineIsoInstrPath}",
+      "--load_compilation_trace=$tracePath",
+      kernelPath,
+    ]);
+    expectOutput("", result4);
+
+    final result5 = await runGenSnapshot("generate patch app snapshot", [
+      "--snapshot_kind=${appSnapshotKind}",
+      "--load_vm_snapshot_data=${coreVMDataPath}",
+      "--load_isolate_snapshot_data=${coreIsoDataPath}",
+      "--isolate_snapshot_data=${patchIsoDataPath}",
+      "--isolate_snapshot_instructions=${patchIsoInstrPath}",
+      "--shared_data=${baselineIsoDataPath}",
+      "--shared_instructions=${baselineIsoInstrPath}",
+      "--load_compilation_trace=$tracePath",
+      kernelPath,
+    ]);
+    expectOutput("", result5);
+  });
+}

runtime/tests/vm/dart/snapshot_test_helper.dart

@@ -41,15 +41,39 @@ void expectOutput(String what, Result result) {
   }
 }
 
-Future<Result> runDartBinary(String prefix, List<String> arguments) async {
-  final binary = Platform.executable;
-  final actualArguments = <String>[]
+final String scriptSuffix = Platform.isWindows ? ".bat" : "";
+final String executableSuffix = Platform.isWindows ? ".exe" : "";
+final String buildDir = p.dirname(Platform.executable);
+final String platformDill = p.join(buildDir, "vm_platform_strong.dill");
+final String genSnapshot = p.join(buildDir, "gen_snapshot${executableSuffix}");
+final String genKernel =
+    p.join("pkg", "vm", "tool", "gen_kernel${scriptSuffix}");
+
+Future<Result> runDart(String prefix, List<String> arguments) {
+  final augmentedArguments = <String>[]
     ..addAll(Platform.executableArguments)
     ..addAll(arguments);
-  print("+ $binary " + actualArguments.join(" "));
-  final processResult = await Process.run(binary, actualArguments);
-  final result = new Result(
-      '[$prefix] ${binary} ${actualArguments.join(' ')}', processResult);
+  return runBinary(prefix, Platform.executable, augmentedArguments);
+}
+
+Future<Result> runGenKernel(String prefix, List<String> arguments) {
+  final augmentedArguments = <String>[]
+    ..add("--platform")
+    ..add(platformDill)
+    ..addAll(arguments);
+  return runBinary(prefix, genKernel, augmentedArguments);
+}
+
+Future<Result> runGenSnapshot(String prefix, List<String> arguments) {
+  return runBinary(prefix, genSnapshot, arguments);
+}
+
+Future<Result> runBinary(
+    String prefix, String binary, List<String> arguments) async {
+  print("+ $binary " + arguments.join(" "));
+  final processResult = await Process.run(binary, arguments);
+  final result =
+      new Result('[$prefix] ${binary} ${arguments.join(' ')}', processResult);
 
   if (processResult.stdout.isNotEmpty) {
     print('''
@@ -72,16 +96,23 @@ ${processResult.stderr}''');
   return result;
 }
 
-Future<Null> checkDeterministicSnapshot(
-    String snapshotKind, String expectedStdout) async {
-  final Directory temp = Directory.systemTemp.createTempSync();
-  final snapshot1Path = p.join(temp.path, 'snapshot1');
-  final snapshot2Path = p.join(temp.path, 'snapshot2');
-
+withTempDir(Future fun(String dir)) async {
+  final Directory tempDir = Directory.systemTemp.createTempSync();
   try {
+    await fun(tempDir.path);
+  } finally {
+    tempDir.deleteSync(recursive: true);
+  }
+}
+
+checkDeterministicSnapshot(String snapshotKind, String expectedStdout) async {
+  await withTempDir((String temp) async {
+    final snapshot1Path = p.join(temp, 'snapshot1');
+    final snapshot2Path = p.join(temp, 'snapshot2');
+
     print("Version ${Platform.version}");
 
-    final generate1Result = await runDartBinary('GENERATE SNAPSHOT 1', [
+    final generate1Result = await runDart('GENERATE SNAPSHOT 1', [
       '--deterministic',
       '--trace_class_finalization',
       '--trace_type_finalization',
@@ -94,7 +125,7 @@ Future<Null> checkDeterministicSnapshot(
     ]);
     expectOutput(expectedStdout, generate1Result);
 
-    final generate2Result = await runDartBinary('GENERATE SNAPSHOT 2', [
+    final generate2Result = await runDart('GENERATE SNAPSHOT 2', [
       '--deterministic',
       '--trace_class_finalization',
       '--trace_type_finalization',
@@ -117,42 +148,36 @@ Future<Null> checkDeterministicSnapshot(
       }
     }
     Expect.equals(snapshot1Bytes.length, snapshot2Bytes.length);
-  } finally {
-    await temp.delete(recursive: true);
-  }
+  });
 }
 
-Future<void> runAppJitTest() async {
-  final Directory temp = Directory.systemTemp.createTempSync();
-  final snapshotPath = p.join(temp.path, 'app.jit');
-  final testPath = Platform.script
-      .toFilePath()
-      .replaceAll(new RegExp(r'_test.dart$'), '_test_body.dart');
+runAppJitTest() async {
+  await withTempDir((String temp) async {
+    final snapshotPath = p.join(temp, 'app.jit');
+    final testPath = Platform.script
+        .toFilePath()
+        .replaceAll(new RegExp(r'_test.dart$'), '_test_body.dart');
 
-  try {
-    final trainingResult = await runDartBinary('TRAINING RUN', [
+    final trainingResult = await runDart('TRAINING RUN', [
       '--snapshot=$snapshotPath',
       '--snapshot-kind=app-jit',
       testPath,
       '--train'
     ]);
     expectOutput("OK(Trained)", trainingResult);
-    final runResult = await runDartBinary('RUN FROM SNAPSHOT', [snapshotPath]);
+    final runResult = await runDart('RUN FROM SNAPSHOT', [snapshotPath]);
     expectOutput("OK(Run)", runResult);
-  } finally {
-    await temp.delete(recursive: true);
-  }
+  });
 }
 
 Future<void> runAppJitBytecodeTest() async {
-  final Directory temp = Directory.systemTemp.createTempSync();
-  final snapshotPath = p.join(temp.path, 'app.jit');
-  final testPath = Platform.script
-      .toFilePath()
-      .replaceAll(new RegExp(r'_test.dart$'), '_test_body.dart');
+  await withTempDir((String temp) async {
+    final snapshotPath = p.join(temp, 'app.jit');
+    final testPath = Platform.script
+        .toFilePath()
+        .replaceAll(new RegExp(r'_test.dart$'), '_test_body.dart');
 
-  try {
-    final trainingResult = await runDartBinary('TRAINING RUN', [
+    final trainingResult = await runDart('TRAINING RUN', [
       '--enable_interpreter',
       '--snapshot=$snapshotPath',
       '--snapshot-kind=app-jit',
@@ -160,10 +185,8 @@ Future<void> runAppJitBytecodeTest() async {
       '--train'
     ]);
     expectOutput("OK(Trained)", trainingResult);
-    final runResult = await runDartBinary(
+    final runResult = await runDart(
         'RUN FROM SNAPSHOT', ['--enable_interpreter', snapshotPath]);
     expectOutput("OK(Run)", runResult);
-  } finally {
-    await temp.delete(recursive: true);
-  }
+  });
 }

runtime/tests/vm/dart/type_feedback_test.dart

@@ -0,0 +1,45 @@
+// 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.
+
+import "dart:async";
+import "dart:io";
+
+import "package:path/path.dart" as p;
+
+import "snapshot_test_helper.dart";
+
+int fib(int n) {
+  if (n <= 1) return 1;
+  return fib(n - 1) + fib(n - 2);
+}
+
+Future<void> main(List<String> args) async {
+  if (args.contains("--child")) {
+    print(fib(35));
+    return;
+  }
+
+  if (!Platform.script.toString().endsWith(".dart")) {
+    print("This test must run from source");
+    return;
+  }
+
+  await withTempDir((String tmp) async {
+    final String feedbackPath = p.join(tmp, "type_feedback.bin");
+
+    final result1 = await runDart("generate type feedback", [
+      "--save_type_feedback=$feedbackPath",
+      Platform.script.toFilePath(),
+      "--child",
+    ]);
+    expectOutput("14930352", result1);
+
+    final result2 = await runDart("use type feedback", [
+      "--load_type_feedback=$feedbackPath",
+      Platform.script.toFilePath(),
+      "--child",
+    ]);
+    expectOutput("14930352", result2);
+  });
+}

runtime/vm/clustered_snapshot.cc

@@ -4251,7 +4251,10 @@ void Serializer::WriteInstructions(RawInstructions* instr, RawCode* code) {
   ASSERT(code != Code::null());
 
   const intptr_t offset = image_writer_->GetTextOffsetFor(instr, code);
-  ASSERT(offset != 0);
+  if (offset == 0) {
+    // Code should have been removed by DropCodeWithoutReusableInstructions.
+    UnexpectedObject(code, "Expected instructions to reuse");
+  }
   Write<int32_t>(offset);
 
   // If offset < 0, it's pointing to a shared instruction. We don't profile

runtime/vm/compiler/jit/compiler.cc

@@ -1058,13 +1058,6 @@ RawObject* Compiler::CompileOptimizedFunction(Thread* thread,
 
   ASSERT(function.ShouldCompilerOptimize());
 
-  // If we are in the optimizing in the mutator/Dart thread, then
-  // this is either an OSR compilation or background compilation is
-  // not currently allowed.
-  ASSERT(!thread->IsMutatorThread() || (osr_id != kNoOSRDeoptId) ||
-         !FLAG_background_compilation ||
-         BackgroundCompiler::IsDisabled(Isolate::Current()) ||
-         !function.is_background_optimizable());
   CompilationPipeline* pipeline =
       CompilationPipeline::New(thread->zone(), function);
   return CompileFunctionHelper(pipeline, function, /* optimized = */ true,

runtime/vm/dart_api_impl.cc

@@ -5943,6 +5943,10 @@ Dart_Handle Dart_LoadCompilationTrace(uint8_t* buffer, intptr_t buffer_length) {
   API_TIMELINE_DURATION(thread);
   DARTSCOPE(thread);
   CHECK_NULL(buffer);
+  Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
+  if (Api::IsError(state)) {
+    return state;
+  }
   CompilationTraceLoader loader(thread);
   const Object& error =
       Object::Handle(loader.CompileTrace(buffer, buffer_length));
@@ -5962,6 +5966,10 @@ Dart_Handle Dart_LoadTypeFeedback(uint8_t* buffer, intptr_t buffer_length) {
   API_TIMELINE_DURATION(thread);
   DARTSCOPE(thread);
   CHECK_NULL(buffer);
+  Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
+  if (Api::IsError(state)) {
+    return state;
+  }
   ReadStream stream(buffer, buffer_length);
   TypeFeedbackLoader loader(thread);
   const Object& error = Object::Handle(loader.LoadFeedback(&stream));

runtime/vm/image_snapshot.cc

@@ -157,8 +157,9 @@ int32_t ImageWriter::GetTextOffsetFor(RawInstructions* instructions,
     ObjectOffsetPair* pair = reuse_instructions_.Lookup(instructions);
     if (pair == NULL) {
       // Code should have been removed by DropCodeWithoutReusableInstructions.
-      FATAL("Expected instructions to reuse\n");
+      return 0;
     }
+    ASSERT(pair->offset != 0);
     return pair->offset;
   }
 
@@ -167,6 +168,7 @@ int32_t ImageWriter::GetTextOffsetFor(RawInstructions* instructions,
     // Negative offsets tell the reader the offset is w/r/t the shared
     // instructions image instead of the app-specific instructions image.
     // Compare ImageReader::GetInstructionsAt.
+    ASSERT(pair->offset != 0);
     return -pair->offset;
   }
 
@@ -175,6 +177,7 @@ int32_t ImageWriter::GetTextOffsetFor(RawInstructions* instructions,
   next_text_offset_ += instructions->HeapSize();
   instructions_.Add(InstructionsData(instructions, code, offset));
 
+  ASSERT(offset != 0);
   return offset;
 }