[ VM / Service ] Add reportLines flag to getSourceReport RPC

Every known user of the coverage report just wants the line numbers. At
the moment they have to do a second RPC to get the Script object, so
they can translate the token positions into line numbers.

Slower test times with coverage are usually caused by the extra time it
takes to run the RPCs. So reporting the line number directly will halve
the time it takes to get coverage, for most users.

Bug: https://github.com/flutter/flutter/issues/86722
Change-Id: I7b8d436669713ebc7b7096790a02593b9cb94dda
TEST=CI
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211081
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

pkg/vm_service/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 7.3.0
+- Update to version `3.51` of the spec.
+- Added optional `reportLines` parameter to `getSourceReport` RPC.
+
 ## 7.1.1
 - Update to version `3.48` of the spec.
 - Added `shows` and `hides` properties to `LibraryDependency`.

pkg/vm_service/java/version.properties

@@ -1 +1 @@
-version=3.49
+version=3.51

pkg/vm_service/lib/src/vm_service.dart

@@ -26,7 +26,7 @@ export 'snapshot_graph.dart'
         HeapSnapshotObjectNoData,
         HeapSnapshotObjectNullData;
 
-const String vmServiceVersion = '3.49.0';
+const String vmServiceVersion = '3.51.0';
 
 /// @optional
 const String optional = 'optional';
@@ -817,6 +817,13 @@ abstract class VmServiceInterface {
   /// compilation error, which could terminate the running Dart program. If this
   /// parameter is not provided, it is considered to have the value `false`.
   ///
+  /// The `reportLines` parameter changes the token positions in
+  /// `SourceReportRange.possibleBreakpoints` and `SourceReportCoverage` to be
+  /// line numbers. This is designed to reduce the number of RPCs that need to
+  /// be performed in the case that the client is only interested in line
+  /// numbers. If this parameter is not provided, it is considered to have the
+  /// value `false`.
+  ///
   /// If `isolateId` refers to an isolate which has exited, then the `Collected`
   /// [Sentinel] is returned.
   ///
@@ -831,6 +838,7 @@ abstract class VmServiceInterface {
     int? tokenPos,
     int? endTokenPos,
     bool? forceCompile,
+    bool? reportLines,
   });
 
   /// The `getVersion` RPC is used to determine what version of the Service
@@ -1429,6 +1437,7 @@ class VmServerConnection {
             tokenPos: params['tokenPos'],
             endTokenPos: params['endTokenPos'],
             forceCompile: params['forceCompile'],
+            reportLines: params['reportLines'],
           );
           break;
         case 'getVersion':
@@ -1929,6 +1938,7 @@ class VmService implements VmServiceInterface {
     int? tokenPos,
     int? endTokenPos,
     bool? forceCompile,
+    bool? reportLines,
   }) =>
       _call('getSourceReport', {
         'isolateId': isolateId,
@@ -1937,6 +1947,7 @@ class VmService implements VmServiceInterface {
         if (tokenPos != null) 'tokenPos': tokenPos,
         if (endTokenPos != null) 'endTokenPos': endTokenPos,
         if (forceCompile != null) 'forceCompile': forceCompile,
+        if (reportLines != null) 'reportLines': reportLines,
       });
 
   @override
@@ -7287,12 +7298,12 @@ class SourceReportCoverage {
   static SourceReportCoverage? parse(Map<String, dynamic>? json) =>
       json == null ? null : SourceReportCoverage._fromJson(json);
 
-  /// A list of token positions in a SourceReportRange which have been executed.
-  /// The list is sorted.
+  /// A list of token positions (or line numbers if reportLines was enabled) in
+  /// a SourceReportRange which have been executed.  The list is sorted.
   List<int>? hits;
 
-  /// A list of token positions in a SourceReportRange which have not been
-  /// executed.  The list is sorted.
+  /// A list of token positions (or line numbers if reportLines was enabled) in
+  /// a SourceReportRange which have not been executed.  The list is sorted.
   List<int>? misses;
 
   SourceReportCoverage({
@@ -7352,9 +7363,9 @@ class SourceReportRange {
   SourceReportCoverage? coverage;
 
   /// Possible breakpoint information for this range, represented as a sorted
-  /// list of token positions.  Provided only when the when the
-  /// PossibleBreakpoint report has been requested and the range has been
-  /// compiled.
+  /// list of token positions (or line numbers if reportLines was enabled).
+  /// Provided only when the when the PossibleBreakpoint report has been
+  /// requested and the range has been compiled.
   @optional
   List<int>? possibleBreakpoints;
 

pkg/vm_service/pubspec.yaml

@@ -3,7 +3,7 @@ description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.
 
-version: 7.1.1
+version: 7.3.0
 
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/vm_service
 

pkg/vm_service/test/coverage_leaf_function_test.dart

@@ -27,9 +27,9 @@ bool allRangesCompiled(coverage) {
   return true;
 }
 
-var tests = <IsolateTest>[
-  hasStoppedAtBreakpoint,
-  (VmService service, IsolateRef isolateRef) async {
+IsolateTest coverageTest(Map<String, dynamic> expectedRange,
+    {required bool reportLines}) {
+  return (VmService service, IsolateRef isolateRef) async {
     final isolateId = isolateRef.id!;
     final isolate = await service.getIsolate(isolateId);
     final stack = await service.getStack(isolateId);
@@ -43,8 +43,31 @@ var tests = <IsolateTest>[
     FuncRef funcRef =
         root.functions!.singleWhere((f) => f.name == 'leafFunction');
     Func func = await service.getObject(isolateId, funcRef.id!) as Func;
+    final location = func.location!;
 
-    final expectedRange = {
+    final report = await service.getSourceReport(
+      isolateId,
+      [SourceReportKind.kCoverage],
+      scriptId: location.script!.id,
+      tokenPos: location.tokenPos,
+      endTokenPos: location.endTokenPos,
+      forceCompile: true,
+      reportLines: reportLines,
+    );
+    expect(report.ranges!.length, 1);
+    expect(report.ranges![0].toJson(), expectedRange);
+    expect(report.scripts!.length, 1);
+    expect(
+      report.scripts![0].uri,
+      endsWith('coverage_leaf_function_test.dart'),
+    );
+  };
+}
+
+var tests = <IsolateTest>[
+  hasStoppedAtBreakpoint,
+  coverageTest(
+    {
       'scriptIndex': 0,
       'startPos': 397,
       'endPos': 447,
@@ -53,39 +76,26 @@ var tests = <IsolateTest>[
         'hits': [],
         'misses': [397]
       }
-    };
-    final location = func.location!;
-
-    final report = await service.getSourceReport(
-        isolateId, [SourceReportKind.kCoverage],
-        scriptId: location.script!.id,
-        tokenPos: location.tokenPos,
-        endTokenPos: location.endTokenPos,
-        forceCompile: true);
-    expect(report.ranges!.length, 1);
-    expect(report.ranges![0].toJson(), expectedRange);
-    expect(report.scripts!.length, 1);
-    expect(
-        report.scripts![0].uri, endsWith('coverage_leaf_function_test.dart'));
-  },
+    },
+    reportLines: false,
+  ),
+  coverageTest(
+    {
+      'scriptIndex': 0,
+      'startPos': 397,
+      'endPos': 447,
+      'compiled': true,
+      'coverage': {
+        'hits': [],
+        'misses': [11]
+      }
+    },
+    reportLines: true,
+  ),
   resumeIsolate,
   hasStoppedAtBreakpoint,
-  (VmService service, IsolateRef isolateRef) async {
-    final isolateId = isolateRef.id!;
-    final isolate = await service.getIsolate(isolateId);
-    final stack = await service.getStack(isolateId);
-
-    // Make sure we are in the right place.
-    expect(stack.frames!.length, greaterThanOrEqualTo(1));
-    expect(stack.frames![0].function!.name, 'testFunction');
-
-    final Library root =
-        await service.getObject(isolateId, isolate.rootLib!.id!) as Library;
-    FuncRef funcRef =
-        root.functions!.singleWhere((f) => f.name == 'leafFunction');
-    Func func = await service.getObject(isolateId, funcRef.id!) as Func;
-
-    var expectedRange = {
+  coverageTest(
+    {
       'scriptIndex': 0,
       'startPos': 397,
       'endPos': 447,
@@ -94,21 +104,22 @@ var tests = <IsolateTest>[
         'hits': [397],
         'misses': []
       }
-    };
-
-    final location = func.location!;
-    final report = await service.getSourceReport(
-        isolateId, [SourceReportKind.kCoverage],
-        scriptId: location.script!.id,
-        tokenPos: location.tokenPos,
-        endTokenPos: location.endTokenPos,
-        forceCompile: true);
-    expect(report.ranges!.length, 1);
-    expect(report.ranges![0].toJson(), expectedRange);
-    expect(report.scripts!.length, 1);
-    expect(
-        report.scripts![0].uri, endsWith('coverage_leaf_function_test.dart'));
-  },
+    },
+    reportLines: false,
+  ),
+  coverageTest(
+    {
+      'scriptIndex': 0,
+      'startPos': 397,
+      'endPos': 447,
+      'compiled': true,
+      'coverage': {
+        'hits': [11],
+        'misses': []
+      }
+    },
+    reportLines: true,
+  ),
 ];
 
 main([args = const <String>[]]) => runIsolateTests(

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'], 3);
-    expect(result['minor'], 50);
+    expect(result['minor'], 51);
     expect(result['_privateMajor'], 0);
     expect(result['_privateMinor'], 0);
   },

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(3));
-    expect(result['minor'], equals(50));
+    expect(result['minor'], equals(51));
     expect(result['_privateMajor'], equals(0));
     expect(result['_privateMinor'], equals(0));
   },

runtime/vm/service.cc

@@ -3353,6 +3353,9 @@ static void GetSourceReport(Thread* thread, JSONStream* js) {
     compile_mode = SourceReport::kForceCompile;
   }
 
+  bool report_lines =
+      BoolParameter::Parse(js->LookupParam("reportLines"), false);
+
   Script& script = Script::Handle();
   intptr_t start_pos = UIntParameter::Parse(js->LookupParam("tokenPos"));
   intptr_t end_pos = UIntParameter::Parse(js->LookupParam("endTokenPos"));
@@ -3383,7 +3386,7 @@ static void GetSourceReport(Thread* thread, JSONStream* js) {
       return;
     }
   }
-  SourceReport report(report_set, compile_mode);
+  SourceReport report(report_set, compile_mode, report_lines);
   report.PrintJSON(js, script, TokenPosition::Deserialize(start_pos),
                    TokenPosition::Deserialize(end_pos));
 #endif  // !DART_PRECOMPILED_RUNTIME

runtime/vm/service.h

@@ -15,7 +15,7 @@
 namespace dart {
 
 #define SERVICE_PROTOCOL_MAJOR_VERSION 3
-#define SERVICE_PROTOCOL_MINOR_VERSION 50
+#define SERVICE_PROTOCOL_MINOR_VERSION 51
 
 class Array;
 class EmbedderServiceHandler;

runtime/vm/service/service.md

@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 3.49
+# Dart VM Service Protocol 3.51
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 3.49_ of the Dart VM Service Protocol. This
+This document describes of _version 3.51_ 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.
@@ -1059,7 +1059,8 @@ SourceReport|Sentinel getSourceReport(string isolateId,
                                       string scriptId [optional],
                                       int tokenPos [optional],
                                       int endTokenPos [optional],
-                                      bool forceCompile [optional])
+                                      bool forceCompile [optional],
+                                      bool reportLines [optional])
 ```
 
 The _getSourceReport_ RPC is used to generate a set of reports tied to
@@ -1095,6 +1096,12 @@ cause a compilation error, which could terminate the running Dart
 program.  If this parameter is not provided, it is considered to have
 the value _false_.
 
+The _reportLines_ parameter changes the token positions in
+_SourceReportRange.possibleBreakpoints_ and _SourceReportCoverage_ to be line
+numbers. This is designed to reduce the number of RPCs that need to be performed
+in the case that the client is only interested in line numbers. If this
+parameter is not provided, it is considered to have the value _false_.
+
 If _isolateId_ refers to an isolate which has exited, then the
 _Collected_ [Sentinel](#sentinel) is returned.
 
@@ -3781,12 +3788,12 @@ locations in an isolate.
 
 ```
 class SourceReportCoverage {
-  // A list of token positions in a SourceReportRange which have been
-  // executed.  The list is sorted.
+  // A list of token positions (or line numbers if reportLines was enabled) in a
+  // SourceReportRange which have been executed.  The list is sorted.
   int[] hits;
 
-  // A list of token positions in a SourceReportRange which have not been
-  // executed.  The list is sorted.
+  // A list of token positions (or line numbers if reportLines was enabled) in a
+  // SourceReportRange which have not been executed.  The list is sorted.
   int[] misses;
 }
 ```
@@ -3836,9 +3843,9 @@ class SourceReportRange {
   SourceReportCoverage coverage [optional];
 
   // Possible breakpoint information for this range, represented as a
-  // sorted list of token positions.  Provided only when the when the
-  // PossibleBreakpoint report has been requested and the range has been
-  // compiled.
+  // sorted list of token positions (or line numbers if reportLines was
+  // enabled).  Provided only when the when the PossibleBreakpoint report has
+  // been requested and the range has been compiled.
   int[] possibleBreakpoints [optional];
 }
 ```
@@ -4176,5 +4183,6 @@ version | comments
 3.48 | Added `Profiler` stream, `UserTagChanged` event kind, and `updatedTag` and `previousTag` properties to `Event`.
 3.49 | Added `CpuSamples` event kind, and `cpuSamples` property to `Event`.
 3.50 | Added `returnType`, `parameters`, and `typeParameters` to `@Instance`, and `implicit` to `@Function`. Added `Parameter` type.
+3.51 | Added optional `reportLines` parameter to `getSourceReport` RPC.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss

runtime/vm/source_report.cc

@@ -22,9 +22,12 @@ const char* SourceReport::kCoverageStr = "Coverage";
 const char* SourceReport::kPossibleBreakpointsStr = "PossibleBreakpoints";
 const char* SourceReport::kProfileStr = "_Profile";
 
-SourceReport::SourceReport(intptr_t report_set, CompileMode compile_mode)
+SourceReport::SourceReport(intptr_t report_set,
+                           CompileMode compile_mode,
+                           bool report_lines)
     : report_set_(report_set),
       compile_mode_(compile_mode),
+      report_lines_(report_lines),
       thread_(NULL),
       script_(NULL),
       start_pos_(TokenPosition::kMinSource),
@@ -218,6 +221,17 @@ void SourceReport::PrintCallSitesData(JSONObject* jsobj,
   }
 }
 
+intptr_t SourceReport::GetTokenPosOrLine(const Script& script,
+                                         const TokenPosition& token_pos) {
+  if (!report_lines_) {
+    return token_pos.Pos();
+  }
+  intptr_t line = -1;
+  const bool found = script.GetTokenLocation(token_pos, &line);
+  ASSERT(found);
+  return line;
+}
+
 void SourceReport::PrintCoverageData(JSONObject* jsobj,
                                      const Function& function,
                                      const Code& code) {
@@ -230,6 +244,7 @@ void SourceReport::PrintCoverageData(JSONObject* jsobj,
   function.RestoreICDataMap(ic_data_array, false /* clone ic-data */);
   const PcDescriptors& descriptors =
       PcDescriptors::Handle(zone(), code.pc_descriptors());
+  const Script& script = Script::Handle(zone(), function.script());
 
   const int kCoverageNone = 0;
   const int kCoverageMiss = 1;
@@ -276,20 +291,24 @@ void SourceReport::PrintCoverageData(JSONObject* jsobj,
   JSONObject cov(jsobj, "coverage");
   {
     JSONArray hits(&cov, "hits");
+    TokenPosition pos = begin_pos;
     for (int i = 0; i < func_length; i++) {
       if (coverage[i] == kCoverageHit) {
-        // Add the token position of the hit.
-        hits.AddValue(begin_pos.Pos() + i);
+        // Add the token position or line number of the hit.
+        hits.AddValue(GetTokenPosOrLine(script, pos));
       }
+      pos = pos.Next();
     }
   }
   {
     JSONArray misses(&cov, "misses");
+    TokenPosition pos = begin_pos;
     for (int i = 0; i < func_length; i++) {
       if (coverage[i] == kCoverageMiss) {
-        // Add the token position of the miss.
-        misses.AddValue(begin_pos.Pos() + i);
+        // Add the token position or line number of the miss.
+        misses.AddValue(GetTokenPosOrLine(script, pos));
       }
+      pos = pos.Next();
     }
   }
 }
@@ -311,6 +330,7 @@ void SourceReport::PrintPossibleBreakpointsData(JSONObject* jsobj,
 
   const PcDescriptors& descriptors =
       PcDescriptors::Handle(zone(), code.pc_descriptors());
+  const Script& script = Script::Handle(zone(), func.script());
 
   PcDescriptors::Iterator iter(descriptors, kSafepointKind);
   while (iter.MoveNext()) {
@@ -324,11 +344,13 @@ void SourceReport::PrintPossibleBreakpointsData(JSONObject* jsobj,
   }
 
   JSONArray bpts(jsobj, "possibleBreakpoints");
+  TokenPosition pos = begin_pos;
   for (int i = 0; i < func_length; i++) {
     if (possible.Contains(i)) {
-      // Add the token position.
-      bpts.AddValue(begin_pos.Pos() + i);
+      // Add the token position or line number.
+      bpts.AddValue(GetTokenPosOrLine(script, pos));
     }
+    pos = pos.Next();
   }
 }
 
@@ -652,7 +674,7 @@ void SourceReport::CollectConstConstructorCoverageFromScripts(
         JSONObject cov(&range, "coverage");
         {
           JSONArray hits(&cov, "hits");
-          hits.AddValue(begin_pos);
+          hits.AddValue(GetTokenPosOrLine(scriptRef, begin_pos));
         }
         {
           JSONArray misses(&cov, "misses");

runtime/vm/source_report.h

@@ -38,7 +38,9 @@ class SourceReport {
 
   // report_set is a bitvector indicating which reports to generate
   // (e.g. kCallSites | kCoverage).
-  explicit SourceReport(intptr_t report_set, CompileMode compile = kNoCompile);
+  explicit SourceReport(intptr_t report_set,
+                        CompileMode compile = kNoCompile,
+                        bool report_lines = false);
   ~SourceReport();
 
   // Generate a source report for (some subrange of) a script.
@@ -66,6 +68,8 @@ class SourceReport {
   bool ShouldSkipField(const Field& field);
   intptr_t GetScriptIndex(const Script& script);
   bool ScriptIsLoadedByLibrary(const Script& script, const Library& lib);
+  intptr_t GetTokenPosOrLine(const Script& script,
+                             const TokenPosition& token_pos);
 
   void PrintCallSitesData(JSONObject* jsobj,
                           const Function& func,
@@ -126,6 +130,7 @@ class SourceReport {
 
   intptr_t report_set_;
   CompileMode compile_mode_;
+  bool report_lines_;
   Thread* thread_;
   const Script* script_;
   TokenPosition start_pos_;