Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2950543002:
Add StartProfiling to filter out v8 warm up (Closed)
DescriptionAdd StartProfiling to filter out v8 warm up
v8.cpu_profiler has a long and variable warm up period. If you look in
tracing that time shows up under V8.Execute which is very misleading. By
adding a tracing category for the time spent in
CpuProfiler::StartProfiling the warm up time can be easily identified or
even filtered out.
With this tracing event the block in StartProfiling correctly described
the time spent prepare v8.cpu_profiler out from V8.Execute
Test by collecting a trace with V8.Execute and look at when a trace
starts generating v8 samples.
BUG=chromium:733853
Review-Url: https://codereview.chromium.org/2950543002
Cr-Commit-Position: refs/heads/master@{#46393}
Committed: https://chromium.googlesource.com/v8/v8/+/93557496a2315111165b1b6f29ef5b5ba60b1837
Patch Set 1 #
Total comments: 3
Patch Set 2 : Add StartProfiling to filter out v8 warm up #
Messages
Total messages: 20 (12 generated)
Description was changed from ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=733853 ========== to ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=733853 ==========
bgirard@fb.com changed reviewers: + alph@chromium.org
https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.cc File src/profiler/cpu-profiler.cc (right): https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.c... src/profiler/cpu-profiler.cc:341: // Enable stack sampling. I suggest to move the event injection point to here, as most of the warmup time is usually spent in LogCompiledFunctions & co.
https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.cc File src/profiler/cpu-profiler.cc (right): https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.c... src/profiler/cpu-profiler.cc:341: // Enable stack sampling. On 2017/06/19 18:39:47, alph wrote: > I suggest to move the event injection point to here, as most of the warmup time > is usually spent in LogCompiledFunctions & co. TRACE_EVENT0 will time all the work happening while it's on the stack which would include the entire runtime of StartProcessorIfNotStarted including the total time of LogCompiledFunctions. Perhaps you're thinking of TRACE_EVENT_INSTANT0 which would let me insert a marker when the execution has been completed? I would prefer to have an event for the entire duration.
https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.cc File src/profiler/cpu-profiler.cc (right): https://codereview.chromium.org/2950543002/diff/1/src/profiler/cpu-profiler.c... src/profiler/cpu-profiler.cc:341: // Enable stack sampling. On 2017/06/19 19:04:32, bgirard wrote: > On 2017/06/19 18:39:47, alph wrote: > > I suggest to move the event injection point to here, as most of the warmup > time > > is usually spent in LogCompiledFunctions & co. > > TRACE_EVENT0 will time all the work happening while it's on the stack which > would include the entire runtime of StartProcessorIfNotStarted including the > total time of LogCompiledFunctions. > > Perhaps you're thinking of TRACE_EVENT_INSTANT0 which would let me insert a > marker when the execution has been completed? I would prefer to have an event > for the entire duration. That makes sense. I thought you wanted to mark a moment when the sampling is really started. lgtm
The CQ bit was checked by bgirard@fb.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/43380)
Description was changed from ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=733853 ========== to ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=chromium:733853 ==========
The CQ bit was checked by bgirard@fb.com
The CQ bit was unchecked by bgirard@fb.com
The CQ bit was checked by bgirard@fb.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by bgirard@fb.com
The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org Link to the patchset: https://codereview.chromium.org/2950543002/#ps20001 (title: "Add StartProfiling to filter out v8 warm up")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1499117723030770,
"parent_rev": "b7a9c0223f4232db7bd525afba676d7e3a34c680", "commit_rev":
"93557496a2315111165b1b6f29ef5b5ba60b1837"}
Message was sent while issue was closed.
Description was changed from ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=chromium:733853 ========== to ========== Add StartProfiling to filter out v8 warm up v8.cpu_profiler has a long and variable warm up period. If you look in tracing that time shows up under V8.Execute which is very misleading. By adding a tracing category for the time spent in CpuProfiler::StartProfiling the warm up time can be easily identified or even filtered out. With this tracing event the block in StartProfiling correctly described the time spent prepare v8.cpu_profiler out from V8.Execute Test by collecting a trace with V8.Execute and look at when a trace starts generating v8 samples. BUG=chromium:733853 Review-Url: https://codereview.chromium.org/2950543002 Cr-Commit-Position: refs/heads/master@{#46393} Committed: https://chromium.googlesource.com/v8/v8/+/93557496a2315111165b1b6f29ef5b5ba60... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/93557496a2315111165b1b6f29ef5b5ba60... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
