[ VM / Service ] Add librariesAlreadyCompiled to getSourceReport RPC

The goal is to have the flutter test framework skip compilation of libraries it's already seen. See option 3 from this doc: https://docs.google.com/document/d/193DKN1DmbHdphhbC8RngdkSafHQGF2QEmf-qDxqSIrk/edit#heading=h.fn9090oeqrk3 Change-Id: Ia76bbb35df22dee6e9f8b32f90bbf2ef2fa18913 TEST=source_report_libraries_already_compiled_test.dart Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/321660 Reviewed-by: Ben Konyi <bkonyi@google.com> Commit-Queue: Liam Appelbe <liama@google.com>
dart-lang · Sep 19, 2023 · 2fcc9bd · 2fcc9bd
1 parent 685af75
commit 2fcc9bd
Show file tree

Hide file tree

Showing 13 changed files with 263 additions and 28 deletions.
diff --git a/pkg/vm_service/CHANGELOG.md b/pkg/vm_service/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 12.0.0
+- Update to version `4.13` of the spec.
+- Add optional `librariesAlreadyCompiled` parameter to `getSourceReport` RPC.
+
 ## 11.10.0
 - Add `wsUri` property to `VmService`. If set, this property can be used to associate
   a `VmService` instance to its targeted VM service based on its URI.

diff --git a/pkg/vm_service/java/version.properties b/pkg/vm_service/java/version.properties
@@ -1 +1 @@
-version=4.12
+version=4.14
diff --git a/pkg/vm_service/lib/src/vm_service.dart b/pkg/vm_service/lib/src/vm_service.dart
@@ -28,7 +28,7 @@ export 'snapshot_graph.dart'
         HeapSnapshotObjectNoData,
         HeapSnapshotObjectNullData;
 
-const String vmServiceVersion = '4.12.0';
+const String vmServiceVersion = '4.13.0';
 
 /// @optional
 const String optional = 'optional';
@@ -975,6 +975,15 @@ abstract class VmServiceInterface {
   /// filter strings. For example, pass `["package:foo/"]` to only include
   /// scripts from the foo package.
   ///
+  /// The `librariesAlreadyCompiled` parameter overrides the `forceCompilation`
+  /// parameter on a per-library basis, setting it to `false` for any libary in
+  /// this list. This is useful for cases where multiple `getSourceReport` RPCs
+  /// are sent with `forceCompilation` enabled, to avoid recompiling the same
+  /// libraries repeatedly. To use this parameter, enable `forceCompilation`,
+  /// cache the results of each `getSourceReport` RPC, and pass all the
+  /// libraries mentioned in the `SourceReport` to subsequent RPCs in the
+  /// `librariesAlreadyCompiled`.
+  ///
   /// If `isolateId` refers to an isolate which has exited, then the `Collected`
   /// [Sentinel] is returned.
   ///
@@ -991,6 +1000,7 @@ abstract class VmServiceInterface {
     bool? forceCompile,
     bool? reportLines,
     List<String>? libraryFilters,
+    List<String>? librariesAlreadyCompiled,
   });
 
   /// The `getVersion` RPC is used to determine what version of the Service
@@ -1699,6 +1709,7 @@ class VmServerConnection {
             forceCompile: params['forceCompile'],
             reportLines: params['reportLines'],
             libraryFilters: params['libraryFilters'],
+            librariesAlreadyCompiled: params['librariesAlreadyCompiled'],
           );
           break;
         case 'getVersion':
@@ -2286,6 +2297,7 @@ class VmService implements VmServiceInterface {
     bool? forceCompile,
     bool? reportLines,
     List<String>? libraryFilters,
+    List<String>? librariesAlreadyCompiled,
   }) =>
       _call('getSourceReport', {
         'isolateId': isolateId,
@@ -2296,6 +2308,8 @@ class VmService implements VmServiceInterface {
         if (forceCompile != null) 'forceCompile': forceCompile,
         if (reportLines != null) 'reportLines': reportLines,
         if (libraryFilters != null) 'libraryFilters': libraryFilters,
+        if (librariesAlreadyCompiled != null)
+          'librariesAlreadyCompiled': librariesAlreadyCompiled,
       });
 
   @override

diff --git a/pkg/vm_service/pubspec.yaml b/pkg/vm_service/pubspec.yaml
@@ -1,5 +1,5 @@
 name: vm_service
-version: 11.10.0
+version: 12.0.0
 description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.

diff --git a/pkg/vm_service/test/source_report_libraries_already_compiled_test.dart b/pkg/vm_service/test/source_report_libraries_already_compiled_test.dart
@@ -0,0 +1,95 @@
+// Copyright (c) 2023, 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:io';
+import 'dart:developer';
+import 'package:test/test.dart';
+import 'package:vm_service/vm_service.dart';
+import 'common/service_test_common.dart';
+import 'common/test_helper.dart';
+
+// ignore_for_file: dead_code
+
+class Class {
+  void method() {
+    print("hit");
+  }
+
+  void missed() {
+    print("miss");
+  }
+}
+
+void unusedFunction() {
+  print("miss");
+}
+
+void testFunction() {
+  if (true) {
+    print("hit");
+    Class().method();
+  } else {
+    print("miss");
+    unusedFunction();
+  }
+  debugger();
+}
+
+const ignoreHitsBelowThisLine = 39;
+
+const allHits = [15, 16, 28, 30, 31, 36];
+final tests = <IsolateTest>[
+  hasStoppedAtBreakpoint,
+  librariesAlreadyCompiledTest(false, [], allHits, [33, 34]),
+  librariesAlreadyCompiledTest(true, [target], allHits, [33, 34]),
+  librariesAlreadyCompiledTest(true, [], allHits, [19, 20, 24, 25, 33, 34]),
+  resumeIsolate,
+];
+
+final target = Platform.script.toString();
+
+librariesAlreadyCompiledTest(
+  bool forceCompile,
+  List<String> librariesAlreadyCompiled,
+  List<int> expectedHits,
+  List<int> expectedMisses,
+) =>
+    (VmService service, IsolateRef isolateRef) async {
+      final isolateId = isolateRef.id!;
+
+      final report = await service.getSourceReport(
+        isolateId,
+        [SourceReportKind.kCoverage],
+        forceCompile: forceCompile,
+        reportLines: true,
+        librariesAlreadyCompiled: librariesAlreadyCompiled,
+      );
+
+      addLines(List<int>? lines, Set<int> out) {
+        for (final line in lines ?? []) {
+          if (line < ignoreHitsBelowThisLine) {
+            out.add(line);
+          }
+        }
+      }
+
+      final hits = <int>{};
+      final misses = <int>{};
+      for (final range in report.ranges!) {
+        if (report.scripts?[range.scriptIndex!].uri == target) {
+          addLines(range.coverage?.hits, hits);
+          addLines(range.coverage?.misses, misses);
+        }
+      }
+
+      expect(hits, unorderedEquals(expectedHits));
+      expect(misses, unorderedEquals(expectedMisses));
+    };
+
+main([args = const <String>[]]) async => await runIsolateTests(
+      args,
+      tests,
+      target,
+      testeeConcurrent: testFunction,
+    );
diff --git a/runtime/observatory/tests/service/get_version_rpc_test.dart b/runtime/observatory/tests/service/get_version_rpc_test.dart
@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], 'Version');
     expect(result['major'], 4);
-    expect(result['minor'], 12);
+    expect(result['minor'], 13);
     expect(result['_privateMajor'], 0);
     expect(result['_privateMinor'], 0);
   },

diff --git a/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart b/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart
@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], equals('Version'));
     expect(result['major'], equals(4));
-    expect(result['minor'], equals(12));
+    expect(result['minor'], equals(13));
     expect(result['_privateMajor'], equals(0));
     expect(result['_privateMinor'], equals(0));
   },

diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
@@ -3620,12 +3620,12 @@ static void GetInstancesAsList(Thread* thread, JSONStream* js) {
   instances.PrintJSON(js, /*ref=*/true);
 }
 
-static intptr_t ParseJSONArray(Thread* thread,
-                               const char* str,
-                               const GrowableObjectArray& elements) {
+template <typename Adder>
+static intptr_t ParseJSONCollection(Thread* thread,
+                                    const char* str,
+                                    const Adder& add) {
   ASSERT(str != nullptr);
   ASSERT(thread != nullptr);
-  Zone* zone = thread->zone();
   intptr_t n = strlen(str);
   if (n < 2) {
     return -1;
@@ -3640,15 +3640,37 @@ static intptr_t ParseJSONArray(Thread* thread,
       // Empty element
       break;
     }
-    String& element = String::Handle(
-        zone, String::FromUTF8(reinterpret_cast<const uint8_t*>(&str[start]),
-                               end - start + 1));
-    elements.Add(element);
+    add(&str[start], end - start + 1);
     start = end + 3;
   }
   return 0;
 }
 
+static intptr_t ParseJSONArray(Thread* thread,
+                               const char* str,
+                               const GrowableObjectArray& elements) {
+  Zone* zone = thread->zone();
+  return ParseJSONCollection(
+      thread, str, [zone, &elements](const char* start, intptr_t length) {
+        String& element = String::Handle(
+            zone,
+            String::FromUTF8(reinterpret_cast<const uint8_t*>(start), length));
+        elements.Add(element);
+      });
+}
+
+#if !defined(DART_PRECOMPILED_RUNTIME)
+static intptr_t ParseJSONSet(Thread* thread,
+                             const char* str,
+                             ZoneCStringSet* elements) {
+  Zone* zone = thread->zone();
+  return ParseJSONCollection(
+      thread, str, [zone, elements](const char* start, intptr_t length) {
+        elements->Insert(zone->MakeCopyOfStringN(start, length));
+      });
+}
+#endif
+
 static const MethodParameter* const get_ports_params[] = {
     RUNNABLE_ISOLATE_PARAMETER,
     nullptr,
@@ -3778,7 +3800,22 @@ static void GetSourceReport(Thread* thread, JSONStream* js) {
     }
   }
 
-  SourceReport report(report_set, library_filters, compile_mode, report_lines);
+  const char* libraries_already_compiled_param =
+      js->LookupParam("librariesAlreadyCompiled");
+  Zone* zone = thread->zone();
+  ZoneCStringSet* libraries_already_compiled = nullptr;
+  if (libraries_already_compiled_param != nullptr) {
+    libraries_already_compiled = new (zone) ZoneCStringSet(zone);
+    intptr_t libraries_already_compiled_length = ParseJSONSet(
+        thread, libraries_already_compiled_param, libraries_already_compiled);
+    if (libraries_already_compiled_length < 0) {
+      PrintInvalidParamError(js, "libraries_already_compiled");
+      return;
+    }
+  }
+
+  SourceReport report(report_set, library_filters, libraries_already_compiled,
+                      compile_mode, report_lines);
   report.PrintJSON(js, script, TokenPosition::Deserialize(start_pos),
                    TokenPosition::Deserialize(end_pos));
 #endif  // !DART_PRECOMPILED_RUNTIME

diff --git a/runtime/vm/service.h b/runtime/vm/service.h
@@ -17,7 +17,7 @@
 namespace dart {
 
 #define SERVICE_PROTOCOL_MAJOR_VERSION 4
-#define SERVICE_PROTOCOL_MINOR_VERSION 12
+#define SERVICE_PROTOCOL_MINOR_VERSION 13
 
 class Array;
 class EmbedderServiceHandler;

diff --git a/runtime/vm/service/service.md b/runtime/vm/service/service.md
@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 4.12
+# Dart VM Service Protocol 4.13
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 4.12_ of the Dart VM Service Protocol. This
+This document describes of _version 4.13_ of the Dart VM Service Protocol. This
 protocol is used to communicate with a running Dart Virtual Machine.
 
 To use the Service Protocol, start the VM with the *--observe* flag.
@@ -1214,7 +1214,8 @@ SourceReport|Sentinel getSourceReport(string isolateId,
                                       int endTokenPos [optional],
                                       bool forceCompile [optional],
                                       bool reportLines [optional],
-                                      string[] libraryFilters [optional])
+                                      string[] libraryFilters [optional],
+                                      string[] librariesAlreadyCompiled [optional])
 ```
 
 The _getSourceReport_ RPC is used to generate a set of reports tied to
@@ -1261,6 +1262,14 @@ for the whole isolate. If it is provided, the _SourceReport_ will only contain
 results from scripts with URIs that start with one of the filter strings. For
 example, pass `["package:foo/"]` to only include scripts from the foo package.
 
+The _librariesAlreadyCompiled_ parameter overrides the _forceCompilation_
+parameter on a per-library basis, setting it to _false_ for any libary in this
+list. This is useful for cases where multiple _getSourceReport_ RPCs are sent
+with _forceCompilation_ enabled, to avoid recompiling the same libraries
+repeatedly. To use this parameter, enable _forceCompilation_, cache the results
+of each _getSourceReport_ RPC, and pass all the libraries mentioned in the
+_SourceReport_ to subsequent RPCs in the _librariesAlreadyCompiled_.
+
 If _isolateId_ refers to an isolate which has exited, then the
 _Collected_ [Sentinel](#sentinel) is returned.
 
@@ -4728,5 +4737,6 @@ version | comments
 4.10 | Deprecated `isSyntheticAsyncContinuation` on `Breakpoint`.
 4.11 | Added `isGetter` and `isSetter` properties to `@Function` and `Function`.
 4.12 | Added `@TypeParameters` and changed `TypeParameters` to extend `Object`.
+4.13 | Added `librariesAlreadyCompiled` to `getSourceReport`.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss