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

Canary memory-infra gives exceptions on ToT #1471

Closed natduca closed 9 years ago

natduca commented 9 years ago

47.0.2510.0 (Official Build) canary (64-bit)

TypeError: Cannot convert undefined or null to object at Function.keys (native) at Object.iterItems (chrome://tracing/tracing.js:340:78) at Object. (chrome://tracing/tracing.js:3259:44) at Object.iterItems (chrome://tracing/tracing.js:340:139) at Object. (chrome://tracing/tracing.js:3252:84) at Array.forEach (native) at Object. (chrome://tracing/tracing.js:3240:188) at Object.iterItems (chrome://tracing/tracing.js:340:139) at Object.TraceEventImporter.createMemoryDumps_ (chrome://tracing/tracing.js:3237:165) at Object.TraceEventImporter.finalizeImport (chrome://tracing/tracing.js:3110:117)

natduca commented 9 years ago

@petrcermak fyi

natduca commented 9 years ago

https://drive.google.com/open?id=0B2UAouCtyY6-cHc0bVdseHRBeVU

petrcermak commented 9 years ago

I assume you are using Yosemite. The issue here is that it uses ".Helvetica Neue DeskInterface" as the system font. This results in the following allocator entries from Skia:

which Chrome interprets nested (because of the dot):

...
'skia/sk_glyph_cache/': {
  'Helvetica Neue DeskInterface_  1/index_20': { 'attrs': A1, 'guid': G1 },
  'Helvetica Neue DeskInterface_  1/index_21': { 'attrs': A2, 'guid': G2 },
  'Helvetica Neue DeskInterface_  1/index_22': { 'attrs': A3, 'guid': G3 }
},
...

instead of flat:

...
'skia/sk_glyph_cache/.Helvetica Neue DeskInterface_  1/index_20': { 'attrs': A1, 'guid': G1 },
'skia/sk_glyph_cache/.Helvetica Neue DeskInterface_  1/index_21': { 'attrs': A2, 'guid': G2 },
'skia/sk_glyph_cache/.Helvetica Neue DeskInterface_  1/index_22': { 'attrs': A3, 'guid': G3 },
...
petrcermak commented 9 years ago

This is a Chrome issue, so I filed a bug for it: https://code.google.com/p/chromium/issues/detail?id=532838

primiano commented 9 years ago

Ok this is due to the silly dot-path-expansion on the chrome base::Value side. There are two issues here: 1) Traceviewer should not crash, just fire a warning and ignore the dump or part of it. Petr has fixed this in crrev.com/1356553002 2) Chrome-side should not auto expand dots. I can fix it easily (crrev.com/1354493003).

@natduca the main question for you (and @dj2 ) is: Can I assume that nothing in tracing is relying on that dot expansion in trace arguments, or would you expect that something is?

primiano commented 9 years ago

Hmm bummer, apparently cc.debug.display_items is depending on the dot expansion already. :-/ I guess the alternative will be normalizing font names in skia

dj2 commented 9 years ago

That sounds like a bug in cc.debug.display_items that we should fix. We shouldn't be depending on that behaviour (mostly because I didn't even know it did that and it seems very surprising).

primiano commented 9 years ago

Ok the fix for that seems easy. See crrev.com/1354493003 Just don't know at this point if we have other instances of this case. ¯(°_o)/¯

natduca commented 9 years ago

Agreed. Fix the idiocy. Dot expansion is @#)$*@# and we shouldn't rely on it anywherez.

On Thu, Sep 17, 2015 at 7:20 AM, Primiano Tucci notifications@github.com wrote:

Ok the fix for that seems easy. See crrev.com/1354493003 Just don't know at this point if we have other instances of this case. ¯(°_o)/¯

— Reply to this email directly or view it on GitHub https://github.com/catapult-project/catapult/issues/1471#issuecomment-141101916 .

petrcermak commented 9 years ago

I'm marking this as fixed (https://codereview.chromium.org/1354493003).