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

Enable sampling profiler for better bootup-time attribution #8526

Open
patrickhulce opened this issue Apr 22, 2019 · 18 comments
Open

Enable sampling profiler for better bootup-time attribution #8526

patrickhulce opened this issue Apr 22, 2019 · 18 comments
Assignees

Comments

@patrickhulce
Copy link
Collaborator

Feature request summary
Enable the disabled-by-default-v8.cpu_profiler trace category.

What is the motivation or use case for changing this?
In #7059 (comment) we discussed enabling the sampling profiler to improve the attribution of scripts in bootup-time audit. Additionally, one of our smoketests that tests attribution was thought to only require async stacks, but turns out also needs sampling profiler now due to other restructuring of tasks.

'bootup-time': {
details: {
items: {
0: {
// TODO: requires async stacks, https://github.com/GoogleChrome/lighthouse/pull/5504
// url: /main-thread-consumer/,
scripting: '>1000',
},
},

How is this beneficial to Lighthouse?
Improved attribution of scripts and the elimination of "Other" as a large category in bootup-time results.

@patrickhulce
Copy link
Collaborator Author

A few other comments in #9200 and #9250 bring up the issue where scripts have parse time but 0ms of execution, which is almost definitely not the case :)

@radical-edo
Copy link

I've found a way to get those numbers. I've been loading the script with type="module". Once I've removed it the lighthouse was able to give me the scripting value for the bootup times. Maybe that's something that will help you to track down the issues with this scripting.

@patrickhulce
Copy link
Collaborator Author

patrickhulce commented Jul 18, 2019

Thanks @radical-edo! That's expected at the moment, #9200 outlined that specific shortcoming and is one of the cases that would be solved by the sampling profiler. It's mainly modules and specific types of async callback work.

@OZZlE
Copy link

OZZlE commented Aug 22, 2019

This really needs to be prioritised I think! This "Other" is the biggest thing on a website I am looking at and we have no clue where it comes from.. Also I couldn't find this flag you mention?
Skärmklipp 2019-08-22 13 59 00

Perhaps it's not a regular flag? Can I enable it somehow on my Chrome anyway?

@OZZlE
Copy link

OZZlE commented Aug 22, 2019

URL Total CPU Time Script Evaluation Script Parse
Other 9,145 ms 1,593 ms 2 ms
-- -- -- --

😞

@patrickhulce
Copy link
Collaborator Author

@OZZlE if by "flag" you're referring to disabled-by-default-v8.cpu_profiler it's a trace category requested over the DevTools protocol, not a Chrome flag. And unfortunately it requires logic to then consume its output, so simply adding the flag wouldn't be enough to workaround your issue.

The lack of movement here is not because we don't want to do it or it's hard to do, it's around the current belief that this trace category would drastically impact the other performance results. Until we can show convincingly otherwise, it will not be implemented.

@connorjclark
Copy link
Collaborator

Until we can show convincingly otherwise, it will not be implemented.

How can we verify the impact of this? DZL??

@patrickhulce
Copy link
Collaborator Author

How can we verify the impact of this? DZL??

Yep, the last time I tried this out in DZL there was no observable impact, but that didn't seem to convince anyone at the time.

@ErisDS
Copy link

ErisDS commented Nov 20, 2019

I'm also having a problem with a site that reports most of the time spent on "Other". Could someone who understands lighthouse a bit explain what known things can end up in this category?

@OZZlE
Copy link

OZZlE commented Nov 20, 2019

Could it be implemented as a Flag? Perhaps enabling it requires a restart of Chrome but I would be fine with it. Even if it affects the over-all scoring of a website, you could at least see where this Other time is coming from; fix that issue, disable the flag and re-run the Audit in Chrome without the flag enabled..

As it is now you could even think that this is just some made-up thing "Other", how do we as web developer know if it's a real thing even right now? When we have no way of seeing what this actually is..

@patrickhulce
Copy link
Collaborator Author

Adding as an option is a neat idea @OZZlE I like it!

@themreza
Copy link

What's the status on this? Has it been released?

@TeoTN
Copy link

TeoTN commented Apr 17, 2020

I'd love to see this ready- my page is running smoothly but it gets poor Performance score with 15seconds in "Others" section, I'm really curious what is hiding there

@patrickhulce patrickhulce self-assigned this Jun 15, 2020
@patrickhulce
Copy link
Collaborator Author

We decided to proceed here and add this data to bootup time in an additive way.

@OZZlE
Copy link

OZZlE commented Jun 17, 2020

@patrickhulce what does that mean? 😕 In PageSpeed I no longer see any "Other" but in Chrome I still see it.. Did you implement any flag we can enable to see what it is?

@patrickhulce
Copy link
Collaborator Author

Did you implement any flag we can enable to see what it is?

That's exactly what this issue is about :)

@connorjclark
Copy link
Collaborator

connorjclark commented Oct 11, 2021

#11608 lays out the next steps:

  • Proactively split continuous looking events
  • Correctly handle negative timeDeltas
  • Interpolate dropped samples
  • Integrate the results into main-thread-tasks computed artifact

At some point (I suggest now?) we should add a CLI flag (default to false) to use this new code path. @brendankenny suggested this is not ideal :)

Let's start validating the new profile model:

  • Look at a few specific use cases using Jquery + $.on(...), $(...), etc, and confirming attribution works out as expected (attributing work done in tasks to the URL that started the task, not the script URL).
  • the lantern tests

@brendankenny
Copy link
Member

I think it could work, just want to define success criteria that we can work toward, rather than a rarely tested LH mode that we can't really comment on beyond "it may be more accurate and might not have excessive impact on other results" :)

Running with and without the extra category and doing a comparison of the difference makes sense as a first step.

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

8 participants