Skip to content

Commit f9e5b92

Browse files
mralephCommit Queue
mraleph
authored and
Commit Queue
committed
Revert "[vm] Intern strings when writing Perfetto timeline"
This reverts commit f2614d2. Reason for revert: Android build broken Original change's description: > [vm] Intern strings when writing Perfetto timeline > > Intern the following fields: > > * Category names > * Event labels > * Debug annotation keys and values > > This significantly reduces the size of the timeline > (e.g. a timeline containing 60k slices goes from 15Mb > to 5Mb timeline) > > TEST=ci and manually > > Change-Id: I59e850279b6714b8b75c5b8e79e6a8b0a981a261 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417202 > Reviewed-by: Derek Xu <[email protected]> No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: I17b83f06ca78fb4b91af554ff4b2aff00366d107 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418024 Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Derek Xu <[email protected]> Commit-Queue: Derek Xu <[email protected]> Auto-Submit: Slava Egorov <[email protected]>
1 parent f07915c commit f9e5b92

23 files changed

+227
-1603
lines changed

‎pkg/vm_service/test/get_perfetto_cpu_samples_rpc_test.dart‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,6 @@ Iterable<PerfSample> extractPerfSamplesFromTracePackets(
6363
final tests = <IsolateTest>[
6464
hasStoppedAtExit,
6565
(VmService service, IsolateRef isolateRef) async {
66-
// This test is calling |getPerfettoCpuSamples| twice: with and without
67-
// filter and expects to get consistent results. However profiler will
68-
// continue running and recording samples while this is happening which
69-
// means the buffer can overflow and old samples will be lost.
70-
//
71-
// The best way to avoid this would be to pause the profiler, but
72-
// service API does not provide a way to do it: setting profiler flag to
73-
// false will stop profiler entirely and discard accumulated samples.
74-
//
75-
// Instead we resort to bumping profile_period to 1h to stabilize this
76-
// test.
77-
await service.setFlag('profile_period', '60000000');
78-
7966
final result = await service.getPerfettoCpuSamples(isolateRef.id!);
8067
expect(result.type, 'PerfettoCpuSamples');
8168
expect(result.samplePeriod, isPositive);
@@ -90,52 +77,18 @@ final tests = <IsolateTest>[
9077
int frameCount = 0;
9178
int callstackCount = 0;
9279
int perfSampleCount = 0;
93-
final seenFrames = <int>{};
94-
final seenCallstacks = <int>{};
9580
for (final packet in packets) {
96-
if (packet.sequenceFlags &
97-
TracePacket_SequenceFlags.SEQ_INCREMENTAL_STATE_CLEARED.value !=
98-
0) {
99-
seenCallstacks.clear();
100-
}
101-
10281
if (packet.hasInternedData()) {
10382
frameCount += packet.internedData.frames.length;
10483
callstackCount += packet.internedData.callstacks.length;
105-
106-
for (var frame in packet.internedData.frames) {
107-
expect(frame.iid.toInt(), isNot(0));
108-
seenFrames.add(frame.iid.toInt());
109-
}
110-
for (var callstack in packet.internedData.callstacks) {
111-
for (var frame in callstack.frameIds) {
112-
expect(frame.toInt(), isIn(seenFrames));
113-
}
114-
115-
expect(callstack.iid.toInt(), isNot(0));
116-
seenCallstacks.add(callstack.iid.toInt());
117-
}
11884
}
11985
if (packet.hasPerfSample()) {
12086
perfSampleCount += 1;
121-
122-
// Check that packet correctly indicates dependency on incremental
123-
// state and that incremental state contains interned callstack
124-
// for this sample.
125-
expect(
126-
packet.sequenceFlags &
127-
TracePacket_SequenceFlags.SEQ_NEEDS_INCREMENTAL_STATE.value !=
128-
0,
129-
isTrue,
130-
);
131-
expect(packet.perfSample.callstackIid.toInt(), isIn(seenCallstacks));
13287
}
13388
}
13489
expect(frameCount, isPositive);
13590
expect(callstackCount, isPositive);
136-
// Note: we will have more perfSampleCount than we have callstacks because
137-
// callstacks are interned.
138-
expect(perfSampleCount, greaterThanOrEqualTo(callstackCount));
91+
expect(perfSampleCount, callstackCount);
13992

14093
// Calculate the time window of events.
14194
final timeOriginNanos = computeTimeOriginNanos(packets);

‎pkg/vm_service/test/get_perfetto_vm_timeline_rpc_test.dart‎

Lines changed: 5 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
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.
4-
//
5-
// VMOptions=
6-
// VMOptions=--intern_strings_when_writing_perfetto_timeline
74

85
import 'dart:collection';
96
import 'dart:convert';
107
import 'dart:developer';
11-
import 'dart:io' show Platform;
128

139
import 'package:test/test.dart';
1410
import 'package:vm_service/vm_service.dart' hide Timeline;
@@ -39,108 +35,12 @@ void primeTimeline() {
3935
Timeline.finishSync();
4036
}
4137

42-
class Deinterner {
43-
final bool stringsShouldBeInterned = Platform.executableArguments
44-
.contains('--intern_strings_when_writing_perfetto_timeline');
45-
46-
final Map<int, String> debugAnnotationNames = {};
47-
final Map<int, String> debugAnnotationStringValues = {};
48-
final Map<int, String> eventNames = {};
49-
final Map<int, String> eventCategories = {};
50-
51-
/// Update the state of the interning dictionaries using [InternedData]
52-
/// from the given packet.
53-
void update(TracePacket packet) {
54-
// Clear the state if [TracePacket.sequenceFlags] instructs us to do so.
55-
if (packet.sequenceFlags &
56-
TracePacket_SequenceFlags.SEQ_INCREMENTAL_STATE_CLEARED.value !=
57-
0) {
58-
debugAnnotationNames.clear();
59-
debugAnnotationStringValues.clear();
60-
eventNames.clear();
61-
eventCategories.clear();
62-
}
63-
64-
if (!packet.hasInternedData()) {
65-
return;
66-
}
67-
68-
final internedData = packet.internedData;
69-
for (var e in internedData.debugAnnotationNames) {
70-
debugAnnotationNames[e.iid.toInt()] = e.name;
71-
}
72-
for (var e in internedData.debugAnnotationStringValues) {
73-
debugAnnotationStringValues[e.iid.toInt()] = utf8.decode(e.str);
74-
}
75-
for (var e in internedData.eventNames) {
76-
eventNames[e.iid.toInt()] = e.name;
77-
}
78-
for (var e in internedData.eventCategories) {
79-
eventCategories[e.iid.toInt()] = e.name;
80-
}
81-
}
82-
83-
/// Deintern contents of the given [TrackEvent].
84-
void deintern(TrackEvent event) {
85-
if (event.hasName()) {
86-
expect(stringsShouldBeInterned, isFalse);
87-
}
88-
if (event.hasNameIid()) {
89-
expect(stringsShouldBeInterned, isTrue);
90-
expect(event.hasName(), isFalse);
91-
event.name = eventNames[event.nameIid.toInt()]!;
92-
event.clearNameIid();
93-
}
94-
95-
if (event.categories.isNotEmpty) {
96-
expect(stringsShouldBeInterned, isFalse);
97-
}
98-
if (event.categoryIids.isNotEmpty) {
99-
expect(stringsShouldBeInterned, isTrue);
100-
expect(event.categories.isEmpty, isTrue);
101-
for (var iid in event.categoryIids) {
102-
event.categories.add(eventCategories[iid.toInt()]!);
103-
}
104-
event.categoryIids.clear();
105-
}
106-
for (var annotation in event.debugAnnotations) {
107-
if (annotation.hasStringValue()) {
108-
expect(stringsShouldBeInterned, isFalse);
109-
}
110-
if (annotation.hasStringValueIid()) {
111-
expect(stringsShouldBeInterned, isTrue);
112-
expect(annotation.hasStringValue(), isFalse);
113-
annotation.stringValue =
114-
debugAnnotationStringValues[annotation.stringValueIid.toInt()]!;
115-
annotation.clearStringValueIid();
116-
}
117-
118-
if (annotation.hasName()) {
119-
expect(stringsShouldBeInterned, isFalse);
120-
}
121-
if (annotation.hasNameIid()) {
122-
expect(stringsShouldBeInterned, isTrue);
123-
expect(annotation.hasName(), isFalse);
124-
annotation.name = debugAnnotationNames[annotation.nameIid.toInt()]!;
125-
annotation.clearNameIid();
126-
}
127-
}
128-
}
129-
}
130-
131-
List<TrackEvent> extractTrackEventsFromTracePackets(
38+
Iterable<TrackEvent> extractTrackEventsFromTracePackets(
13239
List<TracePacket> packets,
13340
) {
134-
final result = <TrackEvent>[];
135-
final deinterner = Deinterner();
136-
for (var packet in packets) {
137-
deinterner.update(packet);
138-
if (packet.hasTrackEvent()) {
139-
deinterner.deintern(packet.trackEvent);
140-
result.add(packet.trackEvent);
141-
}
142-
}
143-
return result;
41+
return packets
42+
.where((packet) => packet.hasTrackEvent())
43+
.map((packet) => packet.trackEvent);
14444
}
14545

14646
Map<String, String> mapFromListOfDebugAnnotations(
@@ -160,7 +60,7 @@ Map<String, String> mapFromListOfDebugAnnotations(
16060
}
16161

16262
void checkThatAllEventsHaveIsolateNumbers(Iterable<TrackEvent> events) {
163-
for (final event in events) {
63+
for (TrackEvent event in events) {
16464
final debugAnnotations =
16565
mapFromListOfDebugAnnotations(event.debugAnnotations);
16666
expect(debugAnnotations['isolateGroupId'], isNotNull);

‎pkg/vm_service_protos/lib/src/protos/perfetto/trace/interned_data/interned_data.pb.dart‎

Lines changed: 21 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ import 'dart:core' as $core;
2121

2222
import 'package:protobuf/protobuf.dart' as $pb;
2323

24-
import '../profiling/profile_common.pb.dart' as $3;
25-
import '../track_event/debug_annotation.pb.dart' as $1;
26-
import '../track_event/track_event.pb.dart' as $2;
24+
import '../profiling/profile_common.pb.dart' as $2;
2725

2826
export 'package:protobuf/protobuf.dart' show GeneratedMessageGenericExtensions;
2927

@@ -38,26 +36,13 @@ export 'package:protobuf/protobuf.dart' show GeneratedMessageGenericExtensions;
3836
/// Next id: 29.
3937
class InternedData extends $pb.GeneratedMessage {
4038
factory InternedData({
41-
$core.Iterable<$2.EventCategory>? eventCategories,
42-
$core.Iterable<$2.EventName>? eventNames,
43-
$core.Iterable<$1.DebugAnnotationName>? debugAnnotationNames,
44-
$core.Iterable<$3.InternedString>? functionNames,
45-
$core.Iterable<$3.Frame>? frames,
46-
$core.Iterable<$3.Callstack>? callstacks,
47-
$core.Iterable<$3.InternedString>? mappingPaths,
48-
$core.Iterable<$3.Mapping>? mappings,
49-
$core.Iterable<$3.InternedString>? debugAnnotationStringValues,
39+
$core.Iterable<$2.InternedString>? functionNames,
40+
$core.Iterable<$2.Frame>? frames,
41+
$core.Iterable<$2.Callstack>? callstacks,
42+
$core.Iterable<$2.InternedString>? mappingPaths,
43+
$core.Iterable<$2.Mapping>? mappings,
5044
}) {
5145
final $result = create();
52-
if (eventCategories != null) {
53-
$result.eventCategories.addAll(eventCategories);
54-
}
55-
if (eventNames != null) {
56-
$result.eventNames.addAll(eventNames);
57-
}
58-
if (debugAnnotationNames != null) {
59-
$result.debugAnnotationNames.addAll(debugAnnotationNames);
60-
}
6146
if (functionNames != null) {
6247
$result.functionNames.addAll(functionNames);
6348
}
@@ -73,9 +58,6 @@ class InternedData extends $pb.GeneratedMessage {
7358
if (mappings != null) {
7459
$result.mappings.addAll(mappings);
7560
}
76-
if (debugAnnotationStringValues != null) {
77-
$result.debugAnnotationStringValues.addAll(debugAnnotationStringValues);
78-
}
7961
return $result;
8062
}
8163
InternedData._() : super();
@@ -91,33 +73,19 @@ class InternedData extends $pb.GeneratedMessage {
9173
package:
9274
const $pb.PackageName(_omitMessageNames ? '' : 'perfetto.protos'),
9375
createEmptyInstance: create)
94-
..pc<$2.EventCategory>(
95-
1, _omitFieldNames ? '' : 'eventCategories', $pb.PbFieldType.PM,
96-
subBuilder: $2.EventCategory.create)
97-
..pc<$2.EventName>(
98-
2, _omitFieldNames ? '' : 'eventNames', $pb.PbFieldType.PM,
99-
subBuilder: $2.EventName.create)
100-
..pc<$1.DebugAnnotationName>(
101-
3, _omitFieldNames ? '' : 'debugAnnotationNames', $pb.PbFieldType.PM,
102-
subBuilder: $1.DebugAnnotationName.create)
103-
..pc<$3.InternedString>(
76+
..pc<$2.InternedString>(
10477
5, _omitFieldNames ? '' : 'functionNames', $pb.PbFieldType.PM,
105-
subBuilder: $3.InternedString.create)
106-
..pc<$3.Frame>(6, _omitFieldNames ? '' : 'frames', $pb.PbFieldType.PM,
107-
subBuilder: $3.Frame.create)
108-
..pc<$3.Callstack>(
78+
subBuilder: $2.InternedString.create)
79+
..pc<$2.Frame>(6, _omitFieldNames ? '' : 'frames', $pb.PbFieldType.PM,
80+
subBuilder: $2.Frame.create)
81+
..pc<$2.Callstack>(
10982
7, _omitFieldNames ? '' : 'callstacks', $pb.PbFieldType.PM,
110-
subBuilder: $3.Callstack.create)
111-
..pc<$3.InternedString>(
83+
subBuilder: $2.Callstack.create)
84+
..pc<$2.InternedString>(
11285
17, _omitFieldNames ? '' : 'mappingPaths', $pb.PbFieldType.PM,
113-
subBuilder: $3.InternedString.create)
114-
..pc<$3.Mapping>(19, _omitFieldNames ? '' : 'mappings', $pb.PbFieldType.PM,
115-
subBuilder: $3.Mapping.create)
116-
..pc<$3.InternedString>(
117-
29,
118-
_omitFieldNames ? '' : 'debugAnnotationStringValues',
119-
$pb.PbFieldType.PM,
120-
subBuilder: $3.InternedString.create)
86+
subBuilder: $2.InternedString.create)
87+
..pc<$2.Mapping>(19, _omitFieldNames ? '' : 'mappings', $pb.PbFieldType.PM,
88+
subBuilder: $2.Mapping.create)
12189
..hasRequiredFields = false;
12290

12391
@$core.Deprecated('Using this can add significant overhead to your binary. '
@@ -143,38 +111,25 @@ class InternedData extends $pb.GeneratedMessage {
143111
$pb.GeneratedMessage.$_defaultFor<InternedData>(create);
144112
static InternedData? _defaultInstance;
145113

146-
@$pb.TagNumber(1)
147-
$pb.PbList<$2.EventCategory> get eventCategories => $_getList(0);
148-
149-
@$pb.TagNumber(2)
150-
$pb.PbList<$2.EventName> get eventNames => $_getList(1);
151-
152-
@$pb.TagNumber(3)
153-
$pb.PbList<$1.DebugAnnotationName> get debugAnnotationNames => $_getList(2);
154-
155114
/// Names of functions used in frames below.
156115
@$pb.TagNumber(5)
157-
$pb.PbList<$3.InternedString> get functionNames => $_getList(3);
116+
$pb.PbList<$2.InternedString> get functionNames => $_getList(0);
158117

159118
/// Frames of callstacks of a program.
160119
@$pb.TagNumber(6)
161-
$pb.PbList<$3.Frame> get frames => $_getList(4);
120+
$pb.PbList<$2.Frame> get frames => $_getList(1);
162121

163122
/// A callstack of a program.
164123
@$pb.TagNumber(7)
165-
$pb.PbList<$3.Callstack> get callstacks => $_getList(5);
124+
$pb.PbList<$2.Callstack> get callstacks => $_getList(2);
166125

167126
/// Paths to executable files.
168127
@$pb.TagNumber(17)
169-
$pb.PbList<$3.InternedString> get mappingPaths => $_getList(6);
128+
$pb.PbList<$2.InternedString> get mappingPaths => $_getList(3);
170129

171130
/// Executable files mapped into processes.
172131
@$pb.TagNumber(19)
173-
$pb.PbList<$3.Mapping> get mappings => $_getList(7);
174-
175-
/// Interned string values in the DebugAnnotation proto.
176-
@$pb.TagNumber(29)
177-
$pb.PbList<$3.InternedString> get debugAnnotationStringValues => $_getList(8);
132+
$pb.PbList<$2.Mapping> get mappings => $_getList(4);
178133
}
179134

180135
const _omitFieldNames = $core.bool.fromEnvironment('protobuf.omit_field_names');

0 commit comments

Comments
 (0)