[ 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>
diff --git a/pkg/vm_service/CHANGELOG.md b/pkg/vm_service/CHANGELOG.md
index 3c1a964..6b3803c 100644
--- a/pkg/vm_service/CHANGELOG.md
+++ b/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`.
diff --git a/pkg/vm_service/java/version.properties b/pkg/vm_service/java/version.properties
index 5e787ed..383674e 100644
--- a/pkg/vm_service/java/version.properties
+++ b/pkg/vm_service/java/version.properties
@@ -1 +1 @@
-version=3.49
+version=3.51
diff --git a/pkg/vm_service/lib/src/vm_service.dart b/pkg/vm_service/lib/src/vm_service.dart
index 77ce904..277b463 100644
--- a/pkg/vm_service/lib/src/vm_service.dart
+++ b/pkg/vm_service/lib/src/vm_service.dart
@@ -26,7 +26,7 @@
HeapSnapshotObjectNoData,
HeapSnapshotObjectNullData;
-const String vmServiceVersion = '3.49.0';
+const String vmServiceVersion = '3.51.0';
/// @optional
const String optional = 'optional';
@@ -817,6 +817,13 @@
/// 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 @@
int? tokenPos,
int? endTokenPos,
bool? forceCompile,
+ bool? reportLines,
});
/// The `getVersion` RPC is used to determine what version of the Service
@@ -1429,6 +1437,7 @@
tokenPos: params['tokenPos'],
endTokenPos: params['endTokenPos'],
forceCompile: params['forceCompile'],
+ reportLines: params['reportLines'],
);
break;
case 'getVersion':
@@ -1929,6 +1938,7 @@
int? tokenPos,
int? endTokenPos,
bool? forceCompile,
+ bool? reportLines,
}) =>
_call('getSourceReport', {
'isolateId': isolateId,
@@ -1937,6 +1947,7 @@
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 @@
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 @@
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;
diff --git a/pkg/vm_service/pubspec.yaml b/pkg/vm_service/pubspec.yaml
index 5aad6b6..85b7cad 100644
--- a/pkg/vm_service/pubspec.yaml
+++ b/pkg/vm_service/pubspec.yaml
@@ -3,7 +3,7 @@
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
diff --git a/pkg/vm_service/test/coverage_leaf_function_test.dart b/pkg/vm_service/test/coverage_leaf_function_test.dart
index fd15551..36b9a88 100644
--- a/pkg/vm_service/test/coverage_leaf_function_test.dart
+++ b/pkg/vm_service/test/coverage_leaf_function_test.dart
@@ -27,9 +27,9 @@
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 @@
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 @@
'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 @@
'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(
diff --git a/runtime/observatory/tests/service/get_version_rpc_test.dart b/runtime/observatory/tests/service/get_version_rpc_test.dart
index 6bec5f1..340b046 100644
--- a/runtime/observatory/tests/service/get_version_rpc_test.dart
+++ b/runtime/observatory/tests/service/get_version_rpc_test.dart
@@ -12,7 +12,7 @@
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);
},
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
index 31490bf..9b7cdf5 100644
--- 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 @@
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));
},
diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
index 4985582..2b43622 100644
--- a/runtime/vm/service.cc
+++ b/runtime/vm/service.cc
@@ -3353,6 +3353,9 @@
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 @@
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
diff --git a/runtime/vm/service.h b/runtime/vm/service.h
index 4c87d36..38583b1 100644
--- a/runtime/vm/service.h
+++ b/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;
diff --git a/runtime/vm/service/service.md b/runtime/vm/service/service.md
index 6e29e0f..5e9b1db 100644
--- a/runtime/vm/service/service.md
+++ b/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 @@
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 @@
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 @@
```
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 @@
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 @@
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
diff --git a/runtime/vm/source_report.cc b/runtime/vm/source_report.cc
index 1bd8a93..f72a72e 100644
--- a/runtime/vm/source_report.cc
+++ b/runtime/vm/source_report.cc
@@ -22,9 +22,12 @@
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 @@
}
}
+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 @@
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 @@
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 @@
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 @@
}
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 @@
JSONObject cov(&range, "coverage");
{
JSONArray hits(&cov, "hits");
- hits.AddValue(begin_pos);
+ hits.AddValue(GetTokenPosOrLine(scriptRef, begin_pos));
}
{
JSONArray misses(&cov, "misses");
diff --git a/runtime/vm/source_report.h b/runtime/vm/source_report.h
index a7acf00..070ca0a 100644
--- a/runtime/vm/source_report.h
+++ b/runtime/vm/source_report.h
@@ -38,7 +38,9 @@
// 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 @@
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 @@
intptr_t report_set_;
CompileMode compile_mode_;
+ bool report_lines_;
Thread* thread_;
const Script* script_;
TokenPosition start_pos_;
