Skip to content

Commit a99edb8

Browse files
mralephcommit-bot@chromium.org
authored andcommitted
[vm_snapshot_analysis] Fix treemap construction
Do not include package nodes names into path when building treemap from ProgramInfo. Library names already contain full package name. Fixes #42907 Fixed: 42907 Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try Change-Id: I91c4fc73edb3b345dfcc485e418d50b2ec5f4fe7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156910 Commit-Queue: Vyacheslav Egorov <[email protected]> Reviewed-by: Kenzie Schmoll <[email protected]>
1 parent 8c5af18 commit a99edb8

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

pkg/vm_snapshot_analysis/lib/treemap.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,15 @@ void _treemapFromInfo(Map<String, dynamic> root, ProgramInfo info,
131131
return;
132132
}
133133

134-
path = path != '' ? '$path/${node.name}' : node.name;
135-
_addSymbol(root, path, '<self>', node.size);
134+
// Don't add package node names to the path because nested library nodes
135+
// already contain package name.
136+
if (node.type == NodeType.packageNode) {
137+
_addSymbol(root, node.name, '<self>', node.size);
138+
} else {
139+
path = path != '' ? '$path/${node.name}' : node.name;
140+
_addSymbol(root, path, '<self>', node.size);
141+
}
142+
136143
for (var child in node.children.values) {
137144
recurse(child, path, root, format);
138145
}

pkg/vm_snapshot_analysis/test/instruction_sizes_test.dart

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:convert';
56
import 'dart:io';
67

78
import 'package:test/test.dart';
89

910
import 'package:vm_snapshot_analysis/instruction_sizes.dart'
1011
as instruction_sizes;
1112
import 'package:vm_snapshot_analysis/program_info.dart';
13+
import 'package:vm_snapshot_analysis/treemap.dart';
1214
import 'package:vm_snapshot_analysis/utils.dart';
1315

1416
import 'utils.dart';
@@ -641,6 +643,40 @@ void main() async {
641643
});
642644
});
643645
});
646+
647+
test('treemap', () async {
648+
await withV8Profile('treemap', testSource, (profileJson) async {
649+
final infoJson = await loadJson(File(profileJson));
650+
final info = await loadProgramInfoFromJson(infoJson,
651+
collapseAnonymousClosures: true);
652+
final treemap = treemapFromInfo(info);
653+
654+
List<Map<String, dynamic>> childrenOf(Map<String, dynamic> node) =>
655+
(node['children'] as List).cast();
656+
657+
String nameOf(Map<String, dynamic> node) => node['n'];
658+
659+
Map<String, dynamic> findChild(Map<String, dynamic> node, String name) {
660+
return childrenOf(node)
661+
.firstWhere((child) => nameOf(child) == name, orElse: () => null);
662+
}
663+
664+
Set<String> childrenNames(Map<String, dynamic> node) {
665+
return childrenOf(node).map(nameOf).toSet();
666+
}
667+
668+
// Verify that we don't include package names twice into paths
669+
// while building the treemap.
670+
if (Platform.isWindows) {
671+
// Note: in Windows we don't consider main.dart part of package:input
672+
// for some reason.
673+
expect(findChild(treemap, 'package:input/input.dart'), isNotNull);
674+
} else {
675+
expect(childrenNames(findChild(treemap, 'package:input')),
676+
equals({'<self>', 'main.dart', 'input.dart'}));
677+
}
678+
});
679+
});
644680
});
645681
}
646682

0 commit comments

Comments
 (0)