Skip to content

Commit de8aa39

Browse files
jensjohaCommit Queue
jensjoha
authored and
Commit Queue
committed
[CFE] Fix leak testing after kernels ast.dart was split into parts
The weekly bot this week finished in half the time, but was green. Turns out the splitting of ast.dart into parts made the actual leak testing not work because `Library` no longer existed in `ast.dart` (but rather in `src/ast/libraries.dart`). This CL: 1) Fixes the issue by also looking up the libraries uri (which is still `ast.dart`. 2) Adds an option for requiring to find instances of some things it looks for (e.g. `Library` in kernel) and throw if it doesn't. This would have made the weekly bot turn red (fail) instead of being green (saying that everything was fine) when really it wasn't. 3) Adds a test that is run on the try bots that will exercise the leak finding - and throw if it doesn't find `Library` in kernel. Change-Id: Ie69bfbd188eb870fdc1e340a341c1271187ef110 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389162 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Jens Johansen <[email protected]>
1 parent 5333f30 commit de8aa39

File tree

9 files changed

+233
-62
lines changed

9 files changed

+233
-62
lines changed

‎pkg/front_end/test/fasta/suite_utils.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import 'package:testing/testing.dart' show Step, TestDescription;
1414
import '../coverage_helper.dart';
1515
import 'testing/suite.dart';
1616

17+
const String limitToArgument = "--limitTo=";
18+
1719
Future<void> internalMain(
1820
CreateContext createContext, {
1921
List<String> arguments = const [],
@@ -25,6 +27,7 @@ Future<void> internalMain(
2527
Logger logger = const StdoutLogger();
2628
List<String>? argumentsTrimmed;
2729
Uri? coverageUri;
30+
int? limitTo;
2831
for (int i = 0; i < arguments.length; i++) {
2932
String argument = arguments[i];
3033
bool trimmed = false;
@@ -42,6 +45,9 @@ Future<void> internalMain(
4245
// Have this 1-indexed when given as an input.
4346
shard = int.parse(argument.substring("--shard=".length)) - 1;
4447
trimmed = true;
48+
} else if (argument.startsWith(limitToArgument)) {
49+
limitTo = int.tryParse(argument.substring(limitToArgument.length));
50+
trimmed = true;
4551
}
4652

4753
if (trimmed && argumentsTrimmed == null) {
@@ -63,6 +69,7 @@ Future<void> internalMain(
6369
configurationPath: configurationPath ?? "../../testing.json",
6470
shards: shards,
6571
shard: shard,
72+
limitTo: limitTo,
6673
logger: logger,
6774
);
6875
if (coverageUri != null) {

‎pkg/front_end/test/flutter_gallery_leak_tester.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ Future<void> main(List<String> args) async {
158158
Uri.parse("package:kernel/ast.dart"),
159159
"Library",
160160
["fileUri"],
161+
expectToAlwaysFind: true,
161162
));
162163
helper.VMServiceHeapHelperSpecificExactLeakFinder heapHelper =
163164
new helper.VMServiceHeapHelperSpecificExactLeakFinder(
@@ -167,6 +168,7 @@ Future<void> main(List<String> args) async {
167168
Uri.parse("package:kernel/ast.dart"),
168169
"Library",
169170
["fileUri", "libraryIdForTesting"],
171+
expectToAlwaysFind: true,
170172
),
171173
],
172174
throwOnPossibleLeak: true,

‎pkg/front_end/test/spell_checking_list_tests.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ ing
437437
inhibit
438438
inlinable
439439
inlineable
440+
inoperable
440441
insights
441442
instrument
442443
instrumenter

‎pkg/front_end/test/vm_service_for_leak_detection.dart‎

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,43 +7,9 @@ import 'dart:io';
77
import "vm_service_heap_helper.dart" as helper;
88

99
Future<void> main(List<String> args) async {
10-
List<helper.Interest> interests = <helper.Interest>[];
11-
interests.add(new helper.Interest(
12-
Uri.parse("package:front_end/src/source/source_library_builder.dart"),
13-
"SourceLibraryBuilder",
14-
["fileUri"],
15-
));
16-
interests.add(new helper.Interest(
17-
Uri.parse("package:front_end/src/source/source_extension_builder.dart"),
18-
"SourceExtensionBuilder",
19-
["extension"],
20-
));
21-
interests.add(new helper.Interest(
22-
Uri.parse("package:kernel/ast.dart"),
23-
"Library",
24-
["fileUri"],
25-
));
26-
interests.add(new helper.Interest(
27-
Uri.parse("package:kernel/ast.dart"),
28-
"Extension",
29-
["name", "fileUri"],
30-
));
31-
32-
helper.VMServiceHeapHelperSpecificExactLeakFinder createNewLeakFinder() =>
33-
new helper.VMServiceHeapHelperSpecificExactLeakFinder(
34-
interests: interests,
35-
prettyPrints: [
36-
new helper.Interest(
37-
Uri.parse("package:kernel/ast.dart"),
38-
"Library",
39-
["fileUri", "libraryIdForTesting"],
40-
),
41-
],
42-
throwOnPossibleLeak: true,
43-
);
44-
10+
List<helper.Interest> interests = getInterests();
4511
helper.VMServiceHeapHelperSpecificExactLeakFinder heapHelper =
46-
createNewLeakFinder();
12+
createNewLeakFinder(interests);
4713

4814
if (args.length > 0 && args[0] == "--dart2js") {
4915
await heapHelper.start([
@@ -79,7 +45,7 @@ Future<void> main(List<String> args) async {
7945
rethrow;
8046
}
8147
print("Will retry in a few seconds.");
82-
heapHelper = createNewLeakFinder();
48+
heapHelper = createNewLeakFinder(interests);
8349
await Future.delayed(const Duration(seconds: 2));
8450
}
8551
}
@@ -100,7 +66,7 @@ Future<void> main(List<String> args) async {
10066
rethrow;
10167
}
10268
print("Will retry in a few seconds.");
103-
heapHelper = createNewLeakFinder();
69+
heapHelper = createNewLeakFinder(interests);
10470
await Future.delayed(const Duration(seconds: 2));
10571
}
10672
}
@@ -114,3 +80,46 @@ Future<void> main(List<String> args) async {
11480
]);
11581
}
11682
}
83+
84+
helper.VMServiceHeapHelperSpecificExactLeakFinder createNewLeakFinder(
85+
List<helper.Interest> interests) {
86+
return new helper.VMServiceHeapHelperSpecificExactLeakFinder(
87+
interests: interests,
88+
prettyPrints: [
89+
new helper.Interest(
90+
Uri.parse("package:kernel/ast.dart"),
91+
"Library",
92+
["fileUri", "libraryIdForTesting"],
93+
expectToAlwaysFind: true,
94+
),
95+
],
96+
throwOnPossibleLeak: true,
97+
);
98+
}
99+
100+
List<helper.Interest> getInterests() {
101+
List<helper.Interest> interests = <helper.Interest>[];
102+
interests.add(new helper.Interest(
103+
Uri.parse("package:front_end/src/source/source_library_builder.dart"),
104+
"SourceLibraryBuilder",
105+
["fileUri"],
106+
));
107+
interests.add(new helper.Interest(
108+
Uri.parse("package:front_end/src/source/source_extension_builder.dart"),
109+
"SourceExtensionBuilder",
110+
["extension"],
111+
));
112+
interests.add(new helper.Interest(
113+
Uri.parse("package:kernel/ast.dart"),
114+
"Library",
115+
["fileUri"],
116+
expectToAlwaysFind: true,
117+
));
118+
interests.add(new helper.Interest(
119+
Uri.parse("package:kernel/ast.dart"),
120+
"Extension",
121+
["name", "fileUri"],
122+
expectToAlwaysFind: true,
123+
));
124+
return interests;
125+
}

‎pkg/front_end/test/vm_service_for_leak_smoke_test.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
7+
import 'fasta/suite_utils.dart' show limitToArgument;
8+
import "vm_service_for_leak_detection.dart" as helper;
9+
10+
Future<void> main(List<String> args) async {
11+
/// Run just a single test from the incremental suite (with finding leaks) to
12+
/// verify that leak finding actually works and that - e.g. a move of kernel
13+
/// stuff - makes it inoperable. Note that we require, for instance,
14+
/// the Library class in the kernel ast to be found.
15+
await helper.createNewLeakFinder(helper.getInterests()).start([
16+
"--enable-asserts",
17+
Platform.script.resolve("incremental_suite.dart").toString(),
18+
"${limitToArgument}1",
19+
"-DaddDebugBreaks=true",
20+
]);
21+
}

‎pkg/front_end/test/vm_service_heap_helper.dart‎

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@ import "vm_service_helper.dart" as vmService;
77
class VMServiceHeapHelperSpecificExactLeakFinder
88
extends vmService.LaunchingVMServiceHelper {
99
final Set _interestsClassNames = {};
10-
final Map<Uri, Map<String, List<String>>> _interests =
11-
new Map<Uri, Map<String, List<String>>>();
12-
final Map<Uri, Map<String, List<String>>> _prettyPrints =
13-
new Map<Uri, Map<String, List<String>>>();
10+
final Map<Uri, Map<String, List<String>>> _interests = {};
11+
final Map<Uri, Map<String, List<String>>> _prettyPrints = {};
12+
final Set<String> _shouldAlwaysFind = {};
1413
final bool throwOnPossibleLeak;
1514
bool verbose = false;
1615
int? timeout;
@@ -22,6 +21,9 @@ class VMServiceHeapHelperSpecificExactLeakFinder
2221
}) {
2322
if (interests.isEmpty) throw "Empty list of interests given";
2423
for (Interest interest in interests) {
24+
if (interest.expectToAlwaysFind) {
25+
_shouldAlwaysFind.add("${interest.uri}|${interest.className}");
26+
}
2527
Map<String, List<String>>? classToFields = _interests[interest.uri];
2628
if (classToFields == null) {
2729
classToFields = Map<String, List<String>>();
@@ -124,28 +126,34 @@ class VMServiceHeapHelperSpecificExactLeakFinder
124126

125127
stopwatch.reset();
126128
List<Leak> leaks = [];
129+
Set<String> leftToAlwaysFind = _shouldAlwaysFind.toSet();
127130
for (vmService.ClassHeapStats member in allocationProfile.members!) {
128131
if (_interestsClassNames.contains(member.classRef!.name)) {
132+
String? libraryId = member.classRef?.library?.id;
133+
if (libraryId == null) continue;
134+
vmService.Library library = (await serviceClient.getObject(
135+
_isolateRef.id!, libraryId)) as vmService.Library;
136+
String? importUriString = library.uri;
137+
if (importUriString == null) continue;
138+
String? classId = member.classRef?.id;
139+
if (classId == null) continue;
129140
vmService.Class c = (await serviceClient.getObject(
130-
_isolateRef.id!, member.classRef!.id!)) as vmService.Class;
131-
String? uriString = c.location?.script?.uri;
132-
if (uriString == null) continue;
133-
Uri uri = Uri.parse(uriString);
134-
Map<String, List<String>>? uriInterest = _interests[uri];
135-
if (uriInterest == null) continue;
136-
List<String>? fieldsForClass = uriInterest[c.name];
137-
if (fieldsForClass == null) continue;
138-
139-
List<String> fieldsForClassPrettyPrint = fieldsForClass;
140-
141-
uriInterest = _prettyPrints[uri];
142-
if (uriInterest != null) {
143-
if (uriInterest[c.name] != null) {
144-
fieldsForClassPrettyPrint = uriInterest[c.name]!;
145-
}
146-
}
141+
_isolateRef.id!, classId)) as vmService.Class;
142+
String? partUriString = c.location?.script?.uri;
143+
if (partUriString == null) continue;
144+
String? className = c.name;
145+
if (className == null) continue;
146+
147+
(List<String>, List<String>)? fieldsData =
148+
_getFieldsForClassAndPrettyPrint(partUriString, className) ??
149+
_getFieldsForClassAndPrettyPrint(importUriString, className);
150+
if (fieldsData == null) continue;
151+
List<String> fieldsForClass = fieldsData.$1;
152+
List<String> fieldsForClassPrettyPrint = fieldsData.$2;
147153

148154
if (member.instancesCurrent != 0) {
155+
leftToAlwaysFind.remove("$partUriString|$className");
156+
leftToAlwaysFind.remove("$importUriString|$className");
149157
if (verbose) {
150158
print("Has ${member.instancesCurrent} instances of "
151159
"${member.classRef!.name}");
@@ -166,6 +174,9 @@ class VMServiceHeapHelperSpecificExactLeakFinder
166174
} else {
167175
noLeakDetected();
168176
}
177+
if (leftToAlwaysFind.isNotEmpty) {
178+
throw "Expected to find, but didn't: $leftToAlwaysFind";
179+
}
169180

170181
print("Looked for leaks in ${stopwatch.elapsedMilliseconds} ms");
171182

@@ -181,6 +192,22 @@ class VMServiceHeapHelperSpecificExactLeakFinder
181192
}
182193
}
183194

195+
(List<String>, List<String>)? _getFieldsForClassAndPrettyPrint(
196+
String uriString, String className) {
197+
Uri uri = Uri.parse(uriString);
198+
Map<String, List<String>>? uriInterest = _interests[uri];
199+
if (uriInterest == null) return null;
200+
List<String>? fieldsForClass = uriInterest[className];
201+
if (fieldsForClass == null) return null;
202+
203+
List<String> fieldsForClassPrettyPrint = fieldsForClass;
204+
uriInterest = _prettyPrints[uri];
205+
if (uriInterest != null && uriInterest[className] != null) {
206+
fieldsForClassPrettyPrint = uriInterest[className]!;
207+
}
208+
return (fieldsForClass, fieldsForClassPrettyPrint);
209+
}
210+
184211
Future<List<Leak>> _findLeaks(
185212
vmService.IsolateRef isolateRef,
186213
vmService.ClassRef classRef,
@@ -364,8 +391,10 @@ class Interest {
364391
final Uri uri;
365392
final String className;
366393
final List<String> fieldNames;
394+
final bool expectToAlwaysFind;
367395

368-
Interest(this.uri, this.className, this.fieldNames);
396+
Interest(this.uri, this.className, this.fieldNames,
397+
{this.expectToAlwaysFind = false});
369398
}
370399

371400
class Leak {

0 commit comments

Comments
 (0)