search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.93k stars 563 forks source link

Shrink recording window #245

Closed catapult-bot closed 9 years ago

catapult-bot commented 9 years ago

Issue by natduca Monday Sep 22, 2014 at 20:08 GMT Originally opened as https://github.com/google/trace-viewer/issues/245

From cawar...@google.com on June 09, 2013 21:39:55

Chrome Version: 28.0.1489.4 Operating System: Chrome OS

URL (if applicable) where crash occurred: chrome://tracing

Can you reproduce this crash? So far, yes.

What steps will reproduce this crash? (or if it's not reproducible, what were you doing just before the crash)?

  1. Go to chrome://tracing
  2. Start, then stop a trace session
  3. Attempt to save the trace file

_DO NOT CHANGE BELOW THIS LINE_ report_id:6c11cb6b29a352b4

Original issue: http://code.google.com/p/trace-viewer/issues/detail?id=239

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From mmenke@chromium.org on June 10, 2013 09:26:23

Looks like a crash in V8 to me.

Cc: eroman@chromium.org
Labels: Cr-Blink-JavaScript

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From eroman@chromium.org on June 10, 2013 10:32:46

Specifically, it was an out-of-memory crash.

I expect it exhausted memory when serializing the tracing data to an in-memory string. (happened when flattening an array to a string).

Probably a working-as-intended, unless the chrome:tracing author can rework things to stream the result to disk instead.

@jamesr: Can you cc the chrome://tracing authors? Thanks!

Cc: jamesr@chromium.org

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From jamesr@chromium.org on June 10, 2013 11:09:54

Cc: nd...@chromium.org pdr@chromium.org scottmg@chromium.org

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on June 10, 2013 11:14:41

@#$@#$@$&#

Cc: dsinclair@chromium.org

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on June 12, 2013 13:17:20

cawarren, was this happening constantly, or just when you had a super long recording?

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on June 12, 2013 13:21:55

Yeah, thats pushing the limits of our recording. 5 seconds is typically what you should shoot for. Yes, annoying. But so many things for us to do. :)

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From cawar...@google.com on June 12, 2013 13:24:34

Okay, good to know - is a trace sample generally sufficient even if the behavior isn't user-visible during the time of the trace?

I ask because this issue (element lag and animation stuttering) was one which was exhibiting sporadically, but consistently within any given 30-60s window.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on June 12, 2013 13:30:26

Nope, you've got to get the stutter in there.

Dan, wdyt about shrinking the recording window size in continuous mode... 500k events per process might be simply too much.

@cawarren, did you have the cc.debug category checked or unchecked?

Summary: Shrink recording window when in continuous recording mode (was: chrome://tracing crashes when attempting to save trace file)
Status: Assigned
Owner: dsinclair@chromium.org

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From cawar...@google.com on June 12, 2013 13:34:17

Checked

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on June 12, 2013 13:44:33

Ah, thats definitely gonna be an issue. Lemme file a separate bug about that.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on July 24, 2013 17:49:36

Labels: -Tool-AboutTracing Feature-AboutTracing

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on August 09, 2013 14:43:22

Broadening. A lot of people record till 100% and it crashes on OOM on save. Of course. We should figure out how to do better.

Summary: Shrink recording window (was: Shrink recording window when in continuous recording mode)

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on August 14, 2013 17:09:14

Status: Available
Owner: ---

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From wangxianzhu@chromium.org on August 14, 2013 17:14:11

Cc: wangxianzhu@chromium.org

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on August 14, 2013 23:46:41

Maybe we can just set the ring size down by 1/4th from now as a stopgap?

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From bauerb@chromium.org on August 20, 2013 09:12:29

Issue chromium:276288 has been merged into this issue.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From bauerb@chromium.org on August 20, 2013 09:13:03

Issue chromium:276288 has been merged into this issue.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From wangxianzhu@chromium.org on August 20, 2013 09:21:06

Submitted the following change in Chromium. Does anyone think we should do more things about it? Should we move this bug back to chromium because the change is on chromium code? r218119 | wangxianzhu@chromium.org | 2013-08-16T23:26:03.746392Z

Changed paths: M http://src.chromium.org/viewvc/chrome/trunk/src/base/debug/trace_event_impl.cc?r1=218119&r2=218118&pathrev=218119 M http://src.chromium.org/viewvc/chrome/trunk/src/base/debug/trace_event_impl.h?r1=218119&r2=218118&pathrev=218119 Shrink trace_event buffers

The original 500000 is way too big. Shrink non-ring-buffer by 1/2 and ring-buffer to 1/4 of non-ring-buffer.

Tests showed that for a busy app with impl-side-painting enabled, the shrunk ring buffer can contain about 3-5 seconds of trace. The time will double after we switch to X event for B/E events.

BUG=248022

Review URL: https://chromiumcodereview.appspot.com/22796009

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From bauerb@chromium.org on August 27, 2013 04:34:19

Issue chromium:168698 has been merged into this issue.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From nd...@chromium.org on August 27, 2013 11:32:15

Lets call this fixed for now and file fresh bugs if we still see people having crashy traces etc. Thank you for attacking this!

(we're keeping chrome tracing related bugs in trace-viewer because @laforge and I couldn't come to consensus on making a label for about:tracing related bugs in chromes bug tracker. -.-)

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From wangxianzhu@chromium.org on August 27, 2013 11:33:49

Status: Fixed

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From pcov...@chromium.org on August 27, 2013 11:34:36

what branch did this go into?

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From wangxianzhu@chromium.org on August 27, 2013 12:18:21

The commit ( r218119 ) is after the m30 branch point ( r217147 ). It's in trunk only.

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From pcov...@google.com on October 25, 2013 09:59:47

wangxianzhu@, are you able to estimate the size in MB of a full ring buffer with this change?

catapult-bot commented 9 years ago

Comment by natduca Monday Sep 22, 2014 at 20:08 GMT

From wangxianzhu@chromium.org on October 25, 2013 11:46:34

sizeof(TraceEvent) is 112 and 84 on 64-bit (LP64) and 32-bit systems, respectively.

Size of a full ring buffer is: 256000 / 4 * 112 = 7,168,000 or 256000 / 4 * 84 = 5,376,000

Size of a full vector buffer is: 256000 * 112 = 28,672,000 or 256000 * 84 = 21,504,000

Owner: wangxianzhu@chromium.org