Skip to content

Commit 7fd4980

Browse files
mralephCommit Queue
authored and
Commit Queue
committed
[vm] Default timeline recorder to nop in PRODUCT
Using ring (or other similar types) of recorders in PRODUCT does not make sense because you can't extract the information they record without vm-service. Additionally "ring" (default recorder type) preallocates ~4Mb of memory to store the data. This CL adds a "none" recorder which simply drops all data and has no memory overhead and makes this default recorder in PRODUCT. We also change code to only allow none, callback, systrace and file recorders in PRODUCT. TEST=manually Bug: b/237989929 Change-Id: I52d6ff8bf766e8fa0969eab0bf2adc45dd9a57c3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261320 Commit-Queue: Slava Egorov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 0ec921e commit 7fd4980

File tree

2 files changed

+79
-36
lines changed

2 files changed

+79
-36
lines changed

runtime/vm/timeline.cc

Lines changed: 70 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@
2424

2525
namespace dart {
2626

27+
#if defined(PRODUCT)
28+
#define DEFAULT_TIMELINE_RECORDER "none"
29+
#define SUPPORTED_TIMELINE_RECORDERS "systrace, file, callback"
30+
#else
31+
#define DEFAULT_TIMELINE_RECORDER "ring"
32+
#define SUPPORTED_TIMELINE_RECORDERS \
33+
"ring, endless, startup, systrace, file, callback"
34+
#endif
35+
2736
DEFINE_FLAG(bool, complete_timeline, false, "Record the complete timeline");
2837
DEFINE_FLAG(bool, startup_timeline, false, "Record the startup timeline");
2938
DEFINE_FLAG(
@@ -45,9 +54,9 @@ DEFINE_FLAG(charp,
4554
"Debugger, Embedder, GC, Isolate, and VM.");
4655
DEFINE_FLAG(charp,
4756
timeline_recorder,
48-
"ring",
57+
DEFAULT_TIMELINE_RECORDER,
4958
"Select the timeline recorder used. "
50-
"Valid values: ring, endless, startup, systrace, file, callback.")
59+
"Valid values: none, " SUPPORTED_TIMELINE_RECORDERS)
5160

5261
// Implementation notes:
5362
//
@@ -94,60 +103,81 @@ DEFINE_FLAG(charp,
94103
std::atomic<bool> RecorderLock::shutdown_lock_ = {false};
95104
std::atomic<intptr_t> RecorderLock::outstanding_event_writes_ = {0};
96105

106+
static TimelineEventRecorder* CreateDefaultTimelineRecorder() {
107+
#if defined(PRODUCT)
108+
return new TimelineEventNopRecorder();
109+
#else
110+
return new TimelineEventRingRecorder();
111+
#endif
112+
}
113+
97114
static TimelineEventRecorder* CreateTimelineRecorder() {
98115
// Some flags require that we use the endless recorder.
99-
const bool use_endless_recorder =
100-
(FLAG_timeline_dir != NULL) || FLAG_complete_timeline;
101116

102-
const bool use_startup_recorder = FLAG_startup_timeline;
103-
const bool use_systrace_recorder = FLAG_systrace_timeline;
104-
const char* flag = FLAG_timeline_recorder;
117+
const char* flag =
118+
FLAG_timeline_recorder != nullptr ? FLAG_timeline_recorder : "";
119+
120+
if (FLAG_systrace_timeline) {
121+
flag = "systrace";
122+
} else if (FLAG_timeline_dir != nullptr || FLAG_complete_timeline) {
123+
flag = "endless";
124+
} else if (FLAG_startup_timeline) {
125+
flag = "startup";
126+
}
105127

106-
if (use_systrace_recorder || (flag != NULL)) {
107-
if (use_systrace_recorder || (strcmp("systrace", flag) == 0)) {
128+
if (strcmp("none", flag) == 0) {
129+
return new TimelineEventNopRecorder();
130+
}
131+
132+
// Systrace recorder.
133+
if (strcmp("systrace", flag) == 0) {
108134
#if defined(DART_HOST_OS_LINUX) || defined(DART_HOST_OS_ANDROID)
109-
return new TimelineEventSystraceRecorder();
135+
return new TimelineEventSystraceRecorder();
110136
#elif defined(DART_HOST_OS_MACOS)
111-
if (__builtin_available(iOS 12.0, macOS 10.14, *)) {
112-
return new TimelineEventMacosRecorder();
113-
}
137+
if (__builtin_available(iOS 12.0, macOS 10.14, *)) {
138+
return new TimelineEventMacosRecorder();
139+
}
114140
#elif defined(DART_HOST_OS_FUCHSIA)
115-
return new TimelineEventFuchsiaRecorder();
141+
return new TimelineEventFuchsiaRecorder();
116142
#else
117-
OS::PrintErr(
118-
"Warning: The systrace timeline recorder is equivalent to the"
119-
"ring recorder on this platform.");
120-
return new TimelineEventRingRecorder();
143+
// Not supported. A warning will be emitted below.
121144
#endif
122-
}
123145
}
124146

125-
if (use_endless_recorder || (flag != NULL)) {
126-
if (use_endless_recorder || (strcmp("endless", flag) == 0)) {
127-
return new TimelineEventEndlessRecorder();
128-
}
147+
if (Utils::StrStartsWith(flag, "file") &&
148+
(flag[4] == '\0' || flag[4] == ':' || flag[4] == '=')) {
149+
const char* filename = flag[4] == '\0' ? "dart-timeline.json" : &flag[5];
150+
return new TimelineEventFileRecorder(filename);
129151
}
130152

131-
if (use_startup_recorder || (flag != NULL)) {
132-
if (use_startup_recorder || (strcmp("startup", flag) == 0)) {
133-
return new TimelineEventStartupRecorder();
134-
}
153+
if (strcmp("callback", flag) == 0) {
154+
return new TimelineEventEmbedderCallbackRecorder();
135155
}
136156

137-
if (strcmp("file", flag) == 0) {
138-
return new TimelineEventFileRecorder("dart-timeline.json");
157+
#if !defined(PRODUCT)
158+
// Recorders below do nothing useful in PRODUCT mode. You can't extract
159+
// information available in them without vm-service.
160+
if (strcmp("endless", flag) == 0) {
161+
return new TimelineEventEndlessRecorder();
139162
}
140-
if (Utils::StrStartsWith(flag, "file:") ||
141-
Utils::StrStartsWith(flag, "file=")) {
142-
return new TimelineEventFileRecorder(&flag[5]);
163+
164+
if (strcmp("startup", flag) == 0) {
165+
return new TimelineEventStartupRecorder();
143166
}
144167

145-
if (strcmp("callback", flag) == 0) {
146-
return new TimelineEventEmbedderCallbackRecorder();
168+
if (strcmp("ring", flag) == 0) {
169+
return new TimelineEventRingRecorder();
147170
}
171+
#endif
148172

149-
// Always fall back to the ring recorder.
150-
return new TimelineEventRingRecorder();
173+
if (strlen(flag) > 0 && strcmp(flag, DEFAULT_TIMELINE_RECORDER) != 0) {
174+
OS::PrintErr(
175+
"Warning: requested %s timeline recorder which is not supported, "
176+
"defaulting to " DEFAULT_TIMELINE_RECORDER " recorder\n",
177+
flag);
178+
}
179+
180+
return CreateDefaultTimelineRecorder();
151181
}
152182

153183
// Returns a caller freed array of stream names in FLAG_timeline_streams.
@@ -1423,6 +1453,10 @@ void TimelineEventEmbedderCallbackRecorder::OnEvent(TimelineEvent* event) {
14231453
callback(&recorder_event);
14241454
}
14251455

1456+
void TimelineEventNopRecorder::OnEvent(TimelineEvent* event) {
1457+
// Do nothing.
1458+
}
1459+
14261460
TimelineEventPlatformRecorder::TimelineEventPlatformRecorder() {}
14271461

14281462
TimelineEventPlatformRecorder::~TimelineEventPlatformRecorder() {}

runtime/vm/timeline.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,15 @@ class TimelineEventEmbedderCallbackRecorder
961961
virtual void OnEvent(TimelineEvent* event);
962962
};
963963

964+
// A recorder that does nothing.
965+
class TimelineEventNopRecorder : public TimelineEventCallbackRecorder {
966+
public:
967+
TimelineEventNopRecorder() {}
968+
virtual ~TimelineEventNopRecorder() {}
969+
970+
virtual void OnEvent(TimelineEvent* event);
971+
};
972+
964973
// A recorder that stores events in chains of blocks of events.
965974
// NOTE: This recorder will continue to allocate blocks until it exhausts
966975
// memory.

0 commit comments

Comments
 (0)