Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the size of the trace #3596

Closed
patrickhulce opened this issue Oct 17, 2017 · 12 comments
Closed

Reduce the size of the trace #3596

patrickhulce opened this issue Oct 17, 2017 · 12 comments

Comments

@patrickhulce
Copy link
Collaborator

patrickhulce commented Oct 17, 2017

Tracing causes many sites on WPT to crash Chrome on the phone. Reducing the required number of categories or streaming these few necessary events is the only path forward to making these failure go away.

Lantern relies heavily on quite a few trace events, isolating the key events necessary, and migrating those to Audits.* domain events to eliminate the need for tracing could be helpful since a majority of time spent is in trace parsing

@patrickhulce patrickhulce changed the title FastMode: Reduce reliance on trace events Lantern: Reduce reliance on trace events Oct 27, 2017
@patrickhulce patrickhulce changed the title Lantern: Reduce reliance on trace events Reduce reliance on trace events Nov 9, 2017
@patrickhulce
Copy link
Collaborator Author

We might want to revisit priority of this since it's responsible for ~10% failure rate on WPT

@tdresser
Copy link

This feels like it's moving towards a world where catapult and lighthouse are less similar, which is seems like a step backwards to me.

Can we accomplish the same goal by splitting some trace categories into finer grained sub-categories?

@patrickhulce
Copy link
Collaborator Author

The long term goal was to enable many of Lighthouse insights on all page runs, not just ones where we can capture a trace. In a way, this is much more aligned with native implementation of TTI.

The short-term goal and realistic version of this is exactly what you describe, moving/copying critical trace events to their own smaller category such that we don't need everything under the sun.

@tdresser
Copy link

Sound good. For metrics like TTI in particular, long term we hope to just spit out the final values into the trace. Perhaps these final values should be in their own trace category?

@paulirish
Copy link
Member

Additionally, tracing causes many sites on WPT to crash Chrome on the phone.

We're also interested in adding another category to the TaskQueueManager::ProcessTaskFromWorkQueue trace event (within Chromium) so that Lighthouse doesn't need to request the toplevel category, which is usually most of the size of the trace:
image

@tdresser
Copy link

What fraction of toplevel is TaskQueueManager::ProcessTaskFromWorkQueue? What else is in toplevel? This might make a bunch of our metrics analysis much easier too.

@tdresser
Copy link

Should the title of this issue be something like "Shrink set of required trace events", or similar?

@paulirish
Copy link
Member

paulirish commented Nov 27, 2017

What fraction of toplevel is TaskQueueManager::ProcessTaskFromWorkQueue?

Here's a 74 mb trace of theverge loading with lighthouse's categories:

image

In the script, i separated ProcessTaskFromWorkQueue into its own category, so the topmost toplevel doesn't include it.
So to answer the question, that event is 21% of toplevel here. Often I see it being less than 20% of the toplevel bytes.

Shrink set of required trace events

probably. right now its including both 1) moving things from the trace to the protocol and 2) shrinking the trace


update feb 2018: CL to tweak trace event category: https://chromium-review.googlesource.com/c/chromium/src/+/791314

@paulirish paulirish added this to the Sprint Siete: January 8-19 milestone Jan 8, 2018
@paulirish paulirish self-assigned this Jan 8, 2018
@paulirish paulirish changed the title Reduce reliance on trace events Reduce the size of the trace Jan 8, 2018
@paulirish
Copy link
Member

paulirish commented Aug 31, 2018

CL updated: https://chromium-review.googlesource.com/c/chromium/src/+/791314

I think we can also get rid of the 'loading' category. probably others.

@paulirish
Copy link
Member

As of r588753 (m71), there's a new event for scheduleable tasks. we can now drop toplevel, and add the new category.

"cat":"disabled-by-default-lighthouse","name":"RunTask",

https://bugs.chromium.org/p/chromium/issues/detail?id=874982#c14

@paulirish
Copy link
Member

just landed:

The two of these dropped one site's trace from 110MB to 48MB. That definitely is the low hanging fruit.


There are probably 2 more obvious improvements we could make.

First, here's what's taking up space (top events, then top categories):
image

  1. screenshots are big. This trace has 398 screenshots. That'll do it. Next is throttling the screenshot capture, i think.
  2. the UpdateLayer event from disabled-by-default-devtools.timeline is SUPER chatty. It's worth hunting down why devtools cares about it. perhaps there's a parent task we can use instead? ALL of them are under 0.3ms long, though 15,000 of them do add up to 376ms. But still....
    image

That's all that stands out to me, the next biggest things don't seem worth it to deal with.

@paulirish
Copy link
Member

We're good for now here. Biggest next item is throttling the screenshots, tracked by #4713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants