Skip to content

Commit

Permalink
Use the new IsolateRef.isolateGroupId to speed up isolate deduping (#…
Browse files Browse the repository at this point in the history 
…422)

* Use isolateRef.isolateGroupId to skip some RPCs

* Update the mock test

* Fix analysis

* Debugging the vm_service version of github CI

* Remove debugging
  • Loading branch information
@liamappelbe
liamappelbe committed Sep 8, 2022
1 parent 63ef320 commit 26b1616
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 64 deletions.
15 changes: 10 additions & 5 deletions lib/src/collect.dart
View file
Expand Up @@ -123,6 +123,7 @@ Future<Map<String, dynamic>> _getAllCoverage(
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'
Expand All @@ -138,16 +139,20 @@ Future<Map<String, dynamic>> _getAllCoverage(
// 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);
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
View file
Expand Up @@ -13,7 +13,7 @@ dependencies:
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
Expand Down
9 changes: 9 additions & 0 deletions test/README.md
View file
@@ -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
```
235 changes: 178 additions & 57 deletions test/collect_coverage_mock_test.dart
View file
Expand Up @@ -10,70 +10,51 @@ import 'package:vm_service/vm_service.dart';

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,
);

Script _script(List<List<int>> tokenPosTable) => Script(
uri: null,
library: null,
id: '',
lineOffset: null,
columnOffset: null,
source: null,
tokenPosTable: tokenPosTable,
);

MockVmService _mockService(int majorVersion, int minorVersion) {
SourceReportRange(scriptIndex: scriptIndex, coverage: coverage);

Script _script(List<List<int>> tokenPosTable) =>
Script(id: 'script', tokenPosTable: tokenPosTable);

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 {
Expand Down Expand Up @@ -251,5 +232,145 @@ void main() {
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'));
});
});
}
2 changes: 1 addition & 1 deletion test/collect_coverage_mock_test.mocks.dart
View file
@@ -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.

Expand Down

1 comment on commit 26b1616

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 26b1616 Previous: ccb70de Ratio
Many isolates - basic coverage 82.21 times slower 56.18 times slower 1.46
Many isolates - function coverage 77.92 times slower 56.64 times slower 1.38
Many isolates - branch coverage 88.73 times slower 64.09 times slower 1.38

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.