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

Trace capture: include JS stacks, exclude netlog #1442

Merged
merged 1 commit into from
Jan 9, 2017
Merged

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented Jan 9, 2017

We can afford to include the JS stacks now, which allows us to start attributing cost at a function level. Moreover, when you view a LH trace in timeline, having the stacks tells fleshes out the flame chart inside of "Evaluate Script" etc.

I've looked at the trace size overhead of various categories and we can trade netlog for cpu_profiler right now for free.

I profiled the load of theverge.com in three modes:

  1. hi-res sampling (cpu_profiler & cpu_profiler.hires)
  2. lo-res sampling (cpu_profiler)
  3. no sampling (no cpu_profiler at all)

image

full details: https://docs.google.com/spreadsheets/d/1T_i2ikSWMH13YzqnKPfrRGjTXrs-C-98jj6x8sipfu0/edit#gid=2071721736&vpid=A6

You can see the size overhead of netlog (which we added randomly without a clear usecase). Flipping that off and turning on cpu_profiler makes sense and shouldn't add much bytesize for general pageload traces.

I've also added latencyInfo which provides the "interaction" part of timeline, aka the "MouseUp" etc details. This has totally negligible filesize (or runtime) overhead.

@brendankenny
Copy link
Member

brendankenny commented Jan 9, 2017

does enabling cpu_profiler.hires affect the other categories or were the differences in the other categories just from expected variance across multiple runs?

@paulirish
Copy link
Member Author

does enabling cpu_profiler.hires affect the other categories or were the differences in the other categories just from expected variance across multiple runs?

doesn't affect the others. yeah that's just variance from multiple runs.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

🍬

@brendankenny brendankenny merged commit b8b519b into master Jan 9, 2017
@brendankenny brendankenny deleted the tracecats branch January 9, 2017 21:46
@wardpeet
Copy link
Collaborator

wardpeet commented Jan 9, 2017

I get this error when running npm start https://airhorner.com
schermafdruk 2017-01-09 23 00 45

I think we had this issue somewhere else as well, trying to find it

Running on canary 57

Lighthouse CLI Launching Chrome... +0ms
  ChromeLauncher Waiting for browser. +128ms
  ChromeLauncher Waiting for browser... +1ms
  ChromeLauncher Waiting for browser..... +510ms
  ChromeLauncher Waiting for browser.....✓ +16ms
  status Initializing… +1s
  status Loading page & waiting for onload +377ms URL, HTTPS, Viewport, ThemeColor, Manifest, Accessibility, ContentWidth
  statusEnd Loading page & waiting for onload +2s
  status Retrieving trace +1ms
  status Disconnecting from browser... +68ms
  ChromeLauncher Killing all Chrome Instances +2ms
Runtime error encountered: SyntaxError: Unexpected number in JSON at position 82724
    at Object.parse (native)
    at onChunkRead (/Volumes/MAC_EXTRA/Projects/tmp/lighthouse/lighthouse-core/gather/driver.js:549:31)
SyntaxError: Unexpected number in JSON at position 82724
    at Object.parse (native)
    at onChunkRead (/Volumes/MAC_EXTRA/Projects/tmp/lighthouse/lighthouse-core/gather/driver.js:549:31)

@paulirish
Copy link
Member Author

Yup. Looks like the startTime in the trace for cpu_profiler is using locale numbers, which naturally breaks the JSON.parse()

image

@a1ph
Copy link
Contributor

a1ph commented Jan 9, 2017

Oh, indeed. I'll fix that.

@a1ph
Copy link
Contributor

a1ph commented Jan 9, 2017

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

Successfully merging this pull request may close these issues.

None yet

4 participants