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

core(driver): deliver trace as events rather than a stream #6056

Merged
merged 6 commits into from
Sep 20, 2018

Conversation

exterkamp
Copy link
Member

@exterkamp exterkamp commented Sep 19, 2018

Summary

This changes the Tracing.start protocol command to use the ReportEvents transferMode instead of ReturnAsStream. This makes the trace gathering much faster, as observed in some automated run timings:

URL ReturnAsStream ReportEvents Difference
http://cnn.com 3410ms 1202ms (2208ms)
http://paulirish.com 174ms 87ms (~86ms)

Note: timings are averages of 5 seperate runs with each method.

Related Issues/PRs

fixes #5968
closes #5939
Based on code in old driver implementation

Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

you can drop the _readTraceFromStream method too

lighthouse-core/gather/driver.js Outdated Show resolved Hide resolved
lighthouse-core/gather/driver.js Outdated Show resolved Hide resolved
lighthouse-core/gather/driver.js Outdated Show resolved Hide resolved
@paulirish paulirish mentioned this pull request Sep 19, 2018
3 tasks
Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

lgtm % brendan's thing


// Issue the command to stop tracing.
return this.sendCommand('Tracing.end').catch(reject);
// setup listener for when a trace bundle is sent
Copy link
Member

Choose a reason for hiding this comment

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

"trace chunk" but yeah. How about "// dataCollected events fire for each trace chunk, in order"

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed, see if u like it.

lighthouse-core/gather/driver.js Outdated Show resolved Hide resolved

return this.sendCommand('IO.read', readArguments).then(onChunkRead);
};
this.once('Tracing.tracingComplete', _ => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

so. much. easier. :D

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.

LGTM!

🐎🏎🚤🏃💨

@paulirish paulirish changed the title core(driver): uses trace transferMode ReportEvents instead of ReturnAsStream core(driver): deliver trace as events rather than a stream Sep 19, 2018
@paulirish paulirish merged commit 1af9f20 into master Sep 20, 2018
@paulirish paulirish deleted the optimize-trace branch September 20, 2018 00:33
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.

Hanging trace retrieval on LR Race trace parsing and chunk fetching
4 participants