Use the new `IsolateRef.isolateGroupId` to speed up isolate deduping (#422)
* Use isolateRef.isolateGroupId to skip some RPCs
* Update the mock test
* Fix analysis
* Debugging the vm_service version of github CI
* Remove debugging
diff --git a/lib/src/collect.dart b/lib/src/collect.dart
index b0df843..f16f3f6 100644
--- a/lib/src/collect.dart
+++ b/lib/src/collect.dart
@@ -123,6 +123,7 @@
final reportLines = _versionCheck(version, 3, 51);
final branchCoverageSupported = _versionCheck(version, 3, 56);
final libraryFilters = _versionCheck(version, 3, 57);
+ final fastIsoGroups = _versionCheck(version, 3, 61);
if (branchCoverage && !branchCoverageSupported) {
branchCoverage = false;
stderr.writeln('Branch coverage was requested, but is not supported'
@@ -138,16 +139,20 @@
// group, otherwise we'll double count the hits.
final isolateOwnerGroup = <String, String>{};
final coveredIsolateGroups = <String>{};
- for (var isolateGroupRef in vm.isolateGroups!) {
- final isolateGroup = await service.getIsolateGroup(isolateGroupRef.id!);
- for (var isolateRef in isolateGroup.isolates!) {
- isolateOwnerGroup[isolateRef.id!] = isolateGroupRef.id!;
+ if (!fastIsoGroups) {
+ for (var isolateGroupRef in vm.isolateGroups!) {
+ final isolateGroup = await service.getIsolateGroup(isolateGroupRef.id!);
+ for (var isolateRef in isolateGroup.isolates!) {
+ isolateOwnerGroup[isolateRef.id!] = isolateGroupRef.id!;
+ }
}
}
for (var isolateRef in vm.isolates!) {
if (isolateIds != null && !isolateIds.contains(isolateRef.id)) continue;
- final isolateGroupId = isolateOwnerGroup[isolateRef.id];
+ final isolateGroupId = fastIsoGroups
+ ? isolateRef.isolateGroupId
+ : isolateOwnerGroup[isolateRef.id];
if (isolateGroupId != null) {
if (coveredIsolateGroups.contains(isolateGroupId)) continue;
coveredIsolateGroups.add(isolateGroupId);
diff --git a/pubspec.yaml b/pubspec.yaml
index c40c923..fc340ee 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -13,7 +13,7 @@
path: ^1.8.0
source_maps: ^0.10.10
stack_trace: ^1.10.0
- vm_service: ^9.2.0
+ vm_service: ^9.4.0
dev_dependencies:
benchmark_harness: ^2.2.0
diff --git a/test/README.md b/test/README.md
new file mode 100644
index 0000000..8fa00df
--- /dev/null
+++ b/test/README.md
@@ -0,0 +1,9 @@
+## Regenerating mocks
+
+Some of the tests use a mock VmService that is automatically generated by
+Mockito. If the VmService changes, run this command in the root directory of
+this repo to regenerate that mock:
+
+```bash
+dart run build_runner build
+```
diff --git a/test/collect_coverage_mock_test.dart b/test/collect_coverage_mock_test.dart
index 162ed9d..3e02e9e 100644
--- a/test/collect_coverage_mock_test.dart
+++ b/test/collect_coverage_mock_test.dart
@@ -10,70 +10,51 @@
import 'collect_coverage_mock_test.mocks.dart';
+@GenerateMocks([VmService])
SourceReportRange _range(int scriptIndex, SourceReportCoverage coverage) =>
- SourceReportRange(
- scriptIndex: scriptIndex,
- startPos: null,
- endPos: null,
- compiled: null,
- error: null,
- coverage: coverage,
- possibleBreakpoints: null,
- branchCoverage: null,
- );
+ SourceReportRange(scriptIndex: scriptIndex, coverage: coverage);
-Script _script(List<List<int>> tokenPosTable) => Script(
- uri: null,
- library: null,
- id: '',
- lineOffset: null,
- columnOffset: null,
- source: null,
- tokenPosTable: tokenPosTable,
- );
+Script _script(List<List<int>> tokenPosTable) =>
+ Script(id: 'script', tokenPosTable: tokenPosTable);
-MockVmService _mockService(int majorVersion, int minorVersion) {
+IsolateRef _isoRef(String id, String isoGroupId) =>
+ IsolateRef(id: id, isolateGroupId: isoGroupId);
+
+IsolateGroupRef _isoGroupRef(String id) => IsolateGroupRef(id: id);
+
+IsolateGroup _isoGroup(String id, List<IsolateRef> isolates) =>
+ IsolateGroup(id: id, isolates: isolates);
+
+MockVmService _mockService(
+ int majorVersion,
+ int minorVersion, {
+ Map<String, List<String>> isolateGroups = const {
+ 'isolateGroup': ['isolate'],
+ },
+}) {
final service = MockVmService();
- final isoRef = IsolateRef(
- id: 'isolate',
- number: null,
- name: null,
- isSystemIsolate: null,
- );
- final isoGroupRef = IsolateGroupRef(
- id: 'isolateGroup',
- number: null,
- name: null,
- isSystemIsolateGroup: null,
- );
- when(service.getVM()).thenAnswer((_) async => VM(
- name: null,
- architectureBits: null,
- hostCPU: null,
- operatingSystem: null,
- targetCPU: null,
- version: null,
- pid: null,
- startTime: null,
- isolates: [isoRef],
- isolateGroups: [isoGroupRef],
- systemIsolates: null,
- systemIsolateGroups: null,
- ));
- when(service.getIsolateGroup('isolateGroup'))
- .thenAnswer((_) async => IsolateGroup(
- id: 'isolateGroup',
- number: null,
- name: null,
- isSystemIsolateGroup: null,
- isolates: [isoRef],
- ));
+ final isoRefs = <IsolateRef>[];
+ final isoGroupRefs = <IsolateGroupRef>[];
+ final isoGroups = <IsolateGroup>[];
+ for (final group in isolateGroups.entries) {
+ isoGroupRefs.add(_isoGroupRef(group.key));
+ final isosOfGroup = <IsolateRef>[];
+ for (final isoId in group.value) {
+ isosOfGroup.add(_isoRef(isoId, group.key));
+ }
+ isoGroups.add(_isoGroup(group.key, isosOfGroup));
+ isoRefs.addAll(isosOfGroup);
+ }
+ when(service.getVM()).thenAnswer(
+ (_) async => VM(isolates: isoRefs, isolateGroups: isoGroupRefs));
+ for (final group in isoGroups) {
+ when(service.getIsolateGroup(group.id)).thenAnswer((_) async => group);
+ }
when(service.getVersion()).thenAnswer(
(_) async => Version(major: majorVersion, minor: minorVersion));
return service;
}
-@GenerateMocks([VmService])
void main() {
group('Mock VM Service', () {
test('Collect coverage', () async {
@@ -251,5 +232,145 @@
expect(result.length, 1);
expect(result['package:foo/foo.dart']?.lineHits, {12: 1, 47: 0});
});
+
+ test('Collect coverage, old isolate group deduping', () async {
+ final service = _mockService(3, 60, isolateGroups: {
+ 'isolateGroupA': ['isolate1', 'isolate2'],
+ 'isolateGroupB': ['isolate3'],
+ });
+ when(service.getSourceReport('isolate1', ['Coverage'],
+ forceCompile: true, reportLines: true))
+ .thenAnswer((_) async => SourceReport(
+ ranges: [
+ _range(
+ 0,
+ SourceReportCoverage(
+ hits: [12],
+ misses: [47],
+ ),
+ ),
+ _range(
+ 1,
+ SourceReportCoverage(
+ hits: [95],
+ misses: [52],
+ ),
+ ),
+ ],
+ scripts: [
+ ScriptRef(
+ uri: 'package:foo/foo.dart',
+ id: 'foo',
+ ),
+ ScriptRef(
+ uri: 'package:bar/bar.dart',
+ id: 'bar',
+ ),
+ ],
+ ));
+ when(service.getSourceReport('isolate3', ['Coverage'],
+ forceCompile: true, reportLines: true))
+ .thenAnswer((_) async => SourceReport(
+ ranges: [
+ _range(
+ 0,
+ SourceReportCoverage(
+ hits: [34],
+ misses: [61],
+ ),
+ ),
+ ],
+ scripts: [
+ ScriptRef(
+ uri: 'package:baz/baz.dart',
+ id: 'baz',
+ ),
+ ],
+ ));
+
+ final jsonResult = await collect(Uri(), false, false, false, null,
+ serviceOverrideForTesting: service);
+ final result = await HitMap.parseJson(
+ jsonResult['coverage'] as List<Map<String, dynamic>>);
+
+ expect(result.length, 3);
+ expect(result['package:foo/foo.dart']?.lineHits, {12: 1, 47: 0});
+ expect(result['package:bar/bar.dart']?.lineHits, {95: 1, 52: 0});
+ expect(result['package:baz/baz.dart']?.lineHits, {34: 1, 61: 0});
+ verifyNever(service.getSourceReport('isolate2', ['Coverage'],
+ forceCompile: true, reportLines: true));
+ verify(service.getIsolateGroup('isolateGroupA'));
+ verify(service.getIsolateGroup('isolateGroupB'));
+ });
+
+ test('Collect coverage, fast isolate group deduping', () async {
+ final service = _mockService(3, 61, isolateGroups: {
+ 'isolateGroupA': ['isolate1', 'isolate2'],
+ 'isolateGroupB': ['isolate3'],
+ });
+ when(service.getSourceReport('isolate1', ['Coverage'],
+ forceCompile: true, reportLines: true))
+ .thenAnswer((_) async => SourceReport(
+ ranges: [
+ _range(
+ 0,
+ SourceReportCoverage(
+ hits: [12],
+ misses: [47],
+ ),
+ ),
+ _range(
+ 1,
+ SourceReportCoverage(
+ hits: [95],
+ misses: [52],
+ ),
+ ),
+ ],
+ scripts: [
+ ScriptRef(
+ uri: 'package:foo/foo.dart',
+ id: 'foo',
+ ),
+ ScriptRef(
+ uri: 'package:bar/bar.dart',
+ id: 'bar',
+ ),
+ ],
+ ));
+ when(service.getSourceReport('isolate3', ['Coverage'],
+ forceCompile: true, reportLines: true))
+ .thenAnswer((_) async => SourceReport(
+ ranges: [
+ _range(
+ 0,
+ SourceReportCoverage(
+ hits: [34],
+ misses: [61],
+ ),
+ ),
+ ],
+ scripts: [
+ ScriptRef(
+ uri: 'package:baz/baz.dart',
+ id: 'baz',
+ ),
+ ],
+ ));
+
+ final jsonResult = await collect(Uri(), false, false, false, null,
+ serviceOverrideForTesting: service);
+ final result = await HitMap.parseJson(
+ jsonResult['coverage'] as List<Map<String, dynamic>>);
+
+ expect(result.length, 3);
+ expect(result['package:foo/foo.dart']?.lineHits, {12: 1, 47: 0});
+ expect(result['package:bar/bar.dart']?.lineHits, {95: 1, 52: 0});
+ expect(result['package:baz/baz.dart']?.lineHits, {34: 1, 61: 0});
+ verifyNever(service.getSourceReport('isolate2', ['Coverage'],
+ forceCompile: true, reportLines: true));
+ verifyNever(service.getIsolateGroup('isolateGroupA'));
+ verifyNever(service.getIsolateGroup('isolateGroupB'));
+ });
});
}
diff --git a/test/collect_coverage_mock_test.mocks.dart b/test/collect_coverage_mock_test.mocks.dart
index 0338186..4ee0971 100644
--- a/test/collect_coverage_mock_test.mocks.dart
+++ b/test/collect_coverage_mock_test.mocks.dart
@@ -1,4 +1,4 @@
-// Mocks generated by Mockito 5.1.0 from annotations
+// Mocks generated by Mockito 5.2.0 from annotations
// in coverage/test/collect_coverage_mock_test.dart.
// Do not manually edit this file.
