[vm] Fix vm_snapshot_analysis logic and tests

* When looking for Code node owner simply always check for synthetic
  owner (referenced through ':owner_' property) first. If it is present
  then it means 'owner_' property is invalid anyway. 25fd0200ef changed
  how WSRs are written into snapshot (instead of writing WSRs we are
  simply writing SMI CIDs) which broke previous logic.
* When building ProgramInfoNode for a Class node accommodate for the
  existence of classes which don't have 'library_' property (e.g. base
  objects like void and dynamic). Additionally, adjust how these base
  objects are written into the snapshot profile so that we preserve
  their names.
* Tweak tests diff and diff-collapse tests to pass, they were failing
  because prologue removal compiler change made some of the diffs so
  tiny that they are ignored by threshold filtering.
* Add two more tests: one that verifies that tracings flags don't have
  any impact on the snapshot size and one that checks we correct
  attribute instructions size to the function node from which these
  instructions originated. This last test is added in anticipation of
  Code object removal to make sure that vm_snapshot_analysis continues
  to work correctly.
* Adjust sorting rules for Code cluster to make it more stable between
  slightly different programs to stabilize output of
  --print-instructions-sizes-to (depends on the sorting algorithm used
  in libc, the test was failing on Mac but not on Linux).

These fixes should unblock relanding of b6dc4dad4d

Fixes #44507
Fixes flutter/flutter#76313

TEST=pkg/vm_snapshot_analysis/test/instruction_sizes_test.dart

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Iee0221dd3a507cb4f36d530404693ebfab3b7bf2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/185826
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
Vyacheslav Egorov
2021-02-21 15:48:27 +00:00
committed by commit-bot@chromium.org
parent 0ed3fb3dee
commit d499f68df5
5 changed files with 154 additions and 73 deletions
+8 -9
View File
@@ -398,11 +398,8 @@ class _ProgramInfoBuilder {
ProgramInfoNode createInfoNodeFor(Node node) {
switch (node.type) {
case 'Code':
var owner = node['owner_'];
var owner = node[':owner_'] ?? node['owner_'];
if (owner.type != 'Type') {
if (owner.type == 'WeakSerializationReference') {
owner = node[':owner_'];
}
final ownerNode =
owner.type == 'Null' ? program.stubs : getInfoNodeFor(owner);
if (owner.type == 'Function') {
@@ -437,13 +434,15 @@ class _ProgramInfoBuilder {
return getInfoNodeFor(node['patched_class_']);
case 'Class':
// Default to root node. Some builtin classes (void, dynamic) don't have
// any information about their library written out.
var ownerNode = program.root;
if (node['library_'] != null) {
return makeInfoNode(node.index,
name: node.name,
parent: getInfoNodeFor(node['library_']) ?? program.root,
type: NodeType.classNode);
ownerNode = getInfoNodeFor(node['library_']) ?? ownerNode;
}
break;
return makeInfoNode(node.index,
name: node.name, parent: ownerNode, type: NodeType.classNode);
case 'Library':
// Create fake owner node for the package which contains this library.
@@ -24,9 +24,9 @@ class K {
@pragma('vm:never-inline')
dynamic makeSomeClosures() {
return [
() => const K(0),
() => const K(1),
() => 11,
() => [const K(0)],
() => [const K(1)],
() => [11],
];
}
@@ -77,10 +77,10 @@ class K {
@pragma('vm:never-inline')
dynamic makeSomeClosures() {
return [
() => const K(0),
() => const K(1),
() => 11,
() => {}, // modified
() => [const K(0)],
() => [const K(1)],
() => [11],
() => [const K(2)], // modified
];
}
@@ -130,7 +130,7 @@ class K {
@pragma('vm:never-inline')
dynamic makeSomeClosures() {
return [
() => const K(0),
() => [const K(0)],
];
}
@@ -202,10 +202,24 @@ void main() async {
// If we are not running from the prebuilt SDK then this test does nothing.
return;
}
group('no-size-impact', () {
for (var flag in [printInstructionSizesFlag, writeV8ProfileFlag]) {
test(flag, () async {
await withFlagImpl(testSource, null, (baseline) async {
await withFlagImpl(testSource, flag, (snapshotWithFlag) async {
final sizeBaseline = File(baseline.outputBinary).statSync().size;
final sizeWithFlag =
File(snapshotWithFlag.outputBinary).statSync().size;
expect(sizeWithFlag - sizeBaseline, equals(0));
});
});
});
}
});
group('instruction-sizes', () {
test('basic-parsing', () async {
await withSymbolSizes('basic-parsing', testSource, (sizesJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
final json = await loadJson(File(sizesJson));
final symbols = instruction_sizes.fromJson(json);
expect(symbols, isNotNull,
@@ -278,8 +292,7 @@ void main() async {
});
test('program-info-from-sizes', () async {
await withSymbolSizes('program-info-from-sizes', testSource,
(sizesJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
final json = await loadJson(File(sizesJson));
final info = loadProgramInfoFromJson(json);
expect(info.root.children, contains('dart:core'));
@@ -327,7 +340,7 @@ void main() async {
});
test('histograms', () async {
await withSymbolSizes('histograms', testSource, (sizesJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
final json = await loadJson(File(sizesJson));
final info = loadProgramInfoFromJson(json);
final bySymbol = computeHistogram(info, HistogramType.bySymbol);
@@ -379,9 +392,8 @@ void main() async {
});
test('diff', () async {
await withSymbolSizes('diff-1', testSource, (sizesJson) async {
await withSymbolSizes('diff-2', testSourceModified,
(modifiedSizesJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
await withSymbolSizes(testSourceModified, (modifiedSizesJson) async {
final infoJson = await loadJson(File(sizesJson));
final info = loadProgramInfoFromJson(infoJson);
final modifiedJson = await loadJson(File(modifiedSizesJson));
@@ -401,7 +413,7 @@ void main() async {
'makeSomeClosures': {
'#type': 'function',
'#size': greaterThan(0), // We added code here.
'<anonymous closure @180>': {
'<anonymous closure @186>': {
'#type': 'function',
'#size': greaterThan(0),
},
@@ -426,9 +438,8 @@ void main() async {
});
test('diff-collapsed', () async {
await withSymbolSizes('diff-collapsed-1', testSource, (sizesJson) async {
await withSymbolSizes('diff-collapsed-2', testSourceModified2,
(modifiedSizesJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
await withSymbolSizes(testSourceModified2, (modifiedSizesJson) async {
final json = await loadJson(File(sizesJson));
final info =
loadProgramInfoFromJson(json, collapseAnonymousClosures: true);
@@ -436,11 +447,17 @@ void main() async {
final modifiedInfo = loadProgramInfoFromJson(modifiedJson,
collapseAnonymousClosures: true);
final diff = computeDiff(info, modifiedInfo);
expect(
diffToJson(diff),
equals({
'#type': 'library',
'@stubs': {
'#type': 'library',
'Type Test Type: int*': {
'#type': 'function',
'#size': lessThan(0),
},
},
'package:input': {
'#type': 'package',
'package:input/input.dart': {
@@ -466,8 +483,7 @@ void main() async {
group('v8-profile', () {
test('program-info-from-profile', () async {
await withV8Profile('program-info-from-profile', testSource,
(profileJson) async {
await withV8Profile(testSource, (profileJson) async {
final infoJson = await loadJson(File(profileJson));
final info = loadProgramInfoFromJson(infoJson);
expect(info.root.children, contains('dart:core'));
@@ -525,8 +541,42 @@ void main() async {
});
});
// This test verifies that we are able to attribute instructions to the
// function they originated from.
test('instruction ownership is preserved', () async {
await withV8Profile(testSource, (profileJson) async {
await withSymbolSizes(testSource, (sizesJson) async {
final fromProfile =
loadProgramInfoFromJson(await loadJson(File(profileJson)));
final fromSymbolSizes = loadProgramInfoFromJson(
await loadJson(File(sizesJson)),
collapseAnonymousClosures: true);
final functionNode = fromProfile.lookup([
'package:input',
'package:input/input.dart',
'::',
'makeSomeClosures'
]);
final codeNode = fromProfile.snapshotInfo.snapshot.nodes.firstWhere(
(n) =>
n.type == 'Code' &&
fromProfile.snapshotInfo.ownerOf(n) == functionNode &&
n.name.contains('makeSomeClosures'));
expect(codeNode['<instructions>'], isNotNull);
final instructionsSize = codeNode['<instructions>'].selfSize;
final symbolSize = fromSymbolSizes.lookup([
'package:input',
'package:input/input.dart',
'',
'makeSomeClosures'
]).size;
expect(instructionsSize - symbolSize, equals(0));
});
});
});
test('histograms', () async {
await withV8Profile('histograms', testSource, (sizesJson) async {
await withV8Profile(testSource, (sizesJson) async {
final infoJson = await loadJson(File(sizesJson));
final info = loadProgramInfoFromJson(infoJson);
final bySymbol = computeHistogram(info, HistogramType.bySymbol);
@@ -578,9 +628,8 @@ void main() async {
});
test('diff', () async {
await withV8Profile('diff-1', testSource, (profileJson) async {
await withV8Profile('diff-2', testSourceModified,
(modifiedProfileJson) async {
await withV8Profile(testSource, (profileJson) async {
await withV8Profile(testSourceModified, (modifiedProfileJson) async {
final infoJson = await loadJson(File(profileJson));
final info = loadProgramInfoFromJson(infoJson);
final modifiedJson = await loadJson(File(modifiedProfileJson));
@@ -599,7 +648,7 @@ void main() async {
'makeSomeClosures': {
'#type': 'function',
'#size': greaterThan(0),
'<anonymous closure @180>': {
'<anonymous closure @186>': {
'#type': 'function',
'#size': greaterThan(0),
},
@@ -624,9 +673,8 @@ void main() async {
});
test('diff-collapsed', () async {
await withV8Profile('diff-collapsed-1', testSource, (profileJson) async {
await withV8Profile('diff-collapsed-2', testSourceModified2,
(modifiedProfileJson) async {
await withV8Profile(testSource, (profileJson) async {
await withV8Profile(testSourceModified2, (modifiedProfileJson) async {
final infoJson = await loadJson(File(profileJson));
final info = loadProgramInfoFromJson(infoJson,
collapseAnonymousClosures: true);
@@ -646,7 +694,6 @@ void main() async {
'#type': 'class',
'makeSomeClosures': {
'#type': 'function',
'#size': lessThan(0),
'<anonymous closure>': {
'#type': 'function',
'#size': lessThan(0),
@@ -661,7 +708,7 @@ void main() async {
});
test('treemap', () async {
await withV8Profile('treemap', testSource, (profileJson) async {
await withV8Profile(testSource, (profileJson) async {
final infoJson = await loadJson(File(profileJson));
final info = await loadProgramInfoFromJson(infoJson,
collapseAnonymousClosures: true);
@@ -695,8 +742,7 @@ void main() async {
});
test('dominators', () async {
await withV8Profile('dominators', chainOfStaticCalls,
(profileJson) async {
await withV8Profile(chainOfStaticCalls, (profileJson) async {
// Note: computing dominators also verifies that we don't have
// unreachable nodes in the snapshot.
final infoJson = await loadJson(File(profileJson));
@@ -709,13 +755,17 @@ void main() async {
});
}
Future withSymbolSizes(String prefix, Map<String, String> source,
Future Function(String sizesJson) f) =>
withFlag(prefix, source, '--print_instructions_sizes_to', f);
final printInstructionSizesFlag = '--print_instructions_sizes_to';
Future withV8Profile(String prefix, Map<String, String> source,
Future Function(String sizesJson) f) =>
withFlag(prefix, source, '--write_v8_snapshot_profile_to', f);
Future withSymbolSizes(
Map<String, String> source, Future Function(String sizesJson) f) =>
withFlag(source, printInstructionSizesFlag, f);
final writeV8ProfileFlag = '--write_v8_snapshot_profile_to';
Future withV8Profile(
Map<String, String> source, Future Function(String sizesJson) f) =>
withFlag(source, writeV8ProfileFlag, f);
// On Windows there is some issue with interpreting entry point URI as a package URI
// it instead gets interpreted as a file URI - which breaks comparison. So we
@@ -69,8 +69,7 @@ void main() async {
group('precompiler-trace', () {
test('basic-parsing', () async {
await withFlag('basic-parsing', testSource, '--trace_precompiler_to',
(json) async {
await withFlag(testSource, '--trace_precompiler_to', (json) async {
final jsonRaw = await loadJson(File(json));
final callGraph = loadTrace(jsonRaw);
callGraph.computeDominators();
@@ -102,9 +101,7 @@ void main() async {
});
test('collapse-by-package', () async {
await withFlag(
'collapse-by-package', testSource, '--trace_precompiler_to',
(json) async {
await withFlag(testSource, '--trace_precompiler_to', (json) async {
final jsonRaw = await loadJson(File(json));
final callGraph = loadTrace(jsonRaw).collapse(NodeType.packageNode);
+34 -13
View File
@@ -20,11 +20,25 @@ final dart2native = () {
return path.canonicalize(dart2native);
}();
Future withFlag(String prefix, Map<String, String> source, String flag,
Future Function(String sizesJson) f) {
return withTempDir(prefix, (dir) async {
final outputBinary = path.join(dir, 'output.exe');
final sizesJson = path.join(dir, 'sizes.json');
class AotSnapshot {
final String outputBinary;
final String sizesJson;
AotSnapshot({this.outputBinary, this.sizesJson});
}
Future withFlag(
Map<String, String> source, String flag, Future Function(String) f) {
return withFlagImpl(source, flag, (info) => f(info.sizesJson));
}
Future withFlagImpl(
Map<String, String> source, String flag, Future Function(AotSnapshot) f) {
return withTempDir((dir) async {
final snapshot = AotSnapshot(
outputBinary: path.join(dir, 'output.exe'),
sizesJson: path.join(dir, 'sizes.json'),
);
final packages = path.join(dir, '.packages');
final mainDart = path.join(dir, 'main.dart');
@@ -41,12 +55,17 @@ import 'package:input/input.dart' as input;
void main(List<String> args) => input.main(args);
''');
final extraGenSnapshotOptions = [
'--dwarf-stack-traces',
if (flag != null) '$flag=${snapshot.sizesJson}',
];
// Compile input.dart to native and output instruction sizes.
final result = await Process.run(dart2native, [
'-o',
outputBinary,
snapshot.outputBinary,
'--packages=$packages',
'--extra-gen-snapshot-options=--dwarf-stack-traces,$flag=$sizesJson',
'--extra-gen-snapshot-options=${extraGenSnapshotOptions.join(',')}',
mainDart,
]);
@@ -56,18 +75,20 @@ Compilation completed successfully.
stdout: ${result.stdout}
stderr: ${result.stderr}
''');
expect(File(outputBinary).existsSync(), isTrue,
expect(File(snapshot.outputBinary).existsSync(), isTrue,
reason: 'Output binary exists');
expect(File(sizesJson).existsSync(), isTrue,
reason: 'Instruction sizes output exists');
if (flag != null) {
expect(File(snapshot.sizesJson).existsSync(), isTrue,
reason: 'Instruction sizes output exists');
}
await f(sizesJson);
await f(snapshot);
});
}
Future withTempDir(String prefix, Future Function(String dir) f) async {
Future withTempDir(Future Function(String dir) f) async {
final tempDir =
Directory.systemTemp.createTempSync('instruction-sizes-test-${prefix}');
Directory.systemTemp.createTempSync('instruction-sizes-test-');
try {
await f(tempDir.path);
} finally {
+20 -6
View File
@@ -1499,18 +1499,28 @@ class CodeSerializationCluster : public SerializationCluster {
struct CodeOrderInfo {
CodePtr code;
intptr_t order;
intptr_t original_index;
};
// We sort code objects in such a way that code objects with the same
// instructions are grouped together. To make sorting more stable between
// similar programs we also sort them further by their original indices -
// this helps to stabilize output of --print-instructions-sizes-to which uses
// the name of the first code object (among those pointing to the same
// instruction objects).
static int CompareCodeOrderInfo(CodeOrderInfo const* a,
CodeOrderInfo const* b) {
if (a->order < b->order) return -1;
if (a->order > b->order) return 1;
if (a->original_index < b->original_index) return -1;
if (a->original_index > b->original_index) return 1;
return 0;
}
static void Insert(GrowableArray<CodeOrderInfo>* order_list,
IntMap<intptr_t>* order_map,
CodePtr code) {
CodePtr code,
intptr_t original_index) {
InstructionsPtr instr = code->untag()->instructions_;
intptr_t key = static_cast<intptr_t>(instr);
intptr_t order;
@@ -1523,6 +1533,7 @@ class CodeSerializationCluster : public SerializationCluster {
CodeOrderInfo info;
info.code = code;
info.order = order;
info.original_index = original_index;
order_list->Add(info);
}
@@ -1530,7 +1541,7 @@ class CodeSerializationCluster : public SerializationCluster {
GrowableArray<CodeOrderInfo> order_list;
IntMap<intptr_t> order_map;
for (intptr_t i = 0; i < codes->length(); i++) {
Insert(&order_list, &order_map, (*codes)[i]);
Insert(&order_list, &order_map, (*codes)[i], i);
}
order_list.Sort(CompareCodeOrderInfo);
ASSERT(order_list.length() == codes->length());
@@ -1543,7 +1554,7 @@ class CodeSerializationCluster : public SerializationCluster {
GrowableArray<CodeOrderInfo> order_list;
IntMap<intptr_t> order_map;
for (intptr_t i = 0; i < codes->length(); i++) {
Insert(&order_list, &order_map, (*codes)[i]->ptr());
Insert(&order_list, &order_map, (*codes)[i]->ptr(), i);
}
order_list.Sort(CompareCodeOrderInfo);
ASSERT(order_list.length() == codes->length());
@@ -5030,11 +5041,14 @@ class VMSerializationRoots : public SerializationRoots {
// Error, CallSiteData has no class object.
if (cid != kErrorCid && cid != kCallSiteDataCid) {
ASSERT(table->HasValidClassAt(cid));
s->AddBaseObject(table->At(cid), "Class");
s->AddBaseObject(
table->At(cid), "Class",
Class::Handle(table->At(cid))
.NameCString(Object::NameVisibility::kInternalName));
}
}
s->AddBaseObject(table->At(kDynamicCid), "Class");
s->AddBaseObject(table->At(kVoidCid), "Class");
s->AddBaseObject(table->At(kDynamicCid), "Class", "dynamic");
s->AddBaseObject(table->At(kVoidCid), "Class", "void");
if (!Snapshot::IncludesCode(s->kind())) {
for (intptr_t i = 0; i < StubCode::NumEntries(); i++) {