8000
Skip to content

core(config): include additional trace categories - #12692

Merged
brendankenny merged 1 commit into
GoogleChrome:masterfrom
victorporof:unified-categories
Jul 5, 2021
Merged

core(config): include additional trace categories#12692
brendankenny merged 1 commit into
GoogleChrome:masterfrom
victorporof:unified-categories

Conversation

@victorporof
Copy link
Copy Markdown
Contributor

Signed-off-by: Victor Porof victorporof@chromium.org

Signed-off-by: Victor Porof <victorporof@chromium.org>
@victorporof
victorporof requested a review from a team as a code owner June 24, 2021 09:51
@victorporof
victorporof requested review from connorjclark and removed request for a team June 24, 2021 09:51
@google-cla google-cla Bot added the cla: yes label Jun 24, 2021
@victorporof victorporof changed the title Include additional trace categories core(config): include additional trace categories Jun 24, 2021
'disabled-by-default-devtools.timeline.frame',
'disabled-by-default-v8.cpu_profiler',
'disabled-by-default-v8.cpu_profiler.hires',
'latencyInfo',
@brendankenny brendankenny Jun 24, 2021
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how much larger do these typically make traces? Will we need to update things like minify-trace to strip new things (or include new things) in the near future?

cc @patrickhulce

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect cpu_profiler.hires to be the largest of these by far on busy pages (which we were planning on enabling anyhow #8526). I think latencyInfo can be fairly chatty too if it's what I think it is.

Updating minify-trace to keep what we need from here, would be great! Could use that process to get some rough data on trace sizes for our common culprits (cnn.com, sfgate.com, theverge.com)

@brendankenny brendankenny left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming no one else has an issue here.

A run on the verge gives a trace of
current Lighthouse - 50.1MiB
this branch - 60.7MiB

So, it's a significant jump (albeit from an already very large base :). Is that going to be an issue for anyone in terms of raw storage?

We'll definitely want to follow up with minify-trace pretty soon, I think.

@victorporof
victorporof commented Jul 5, 2021
Copy link
Copy Markdown
Contributor Author

Filed #12748 for updating minify-trace.

The test failure is a known intermittent.

@brendankenny
Copy link
Copy Markdown
Contributor

Is that going to be an issue for anyone in terms of raw storage?

Enough time for objections has probably passed :) we can just keep an eye out for any issues

@brendankenny
brendankenny merged commit 175619a into GoogleChrome:master Jul 5, 2021
@victorporof
victorporof deleted the unified-categories branch July 6, 2021 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0