Skip to content

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

Merged
paulirish merged 6 commits into
masterfrom
optimize-trace
Sep 20, 2018
Merged

paulirish merged 6 commits into
masterfrom
optimize-trace

Conversation

@exterkamp

@exterkamp exterkamp commented Sep 19, 2018 •

Copy link
Copy Markdown
Member

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

Comment thread lighthouse-core/gather/driver.js Outdated

@paulirish paulirish left a comment

Copy link
Copy Markdown
Member

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

Comment thread lighthouse-core/gather/driver.js Outdated
Comment thread lighthouse-core/gather/driver.js Outdated
Comment thread lighthouse-core/gather/driver.js Outdated
Comment thread lighthouse-core/audits/accessibility/axe-audit.js
Comment thread lighthouse-core/gather/driver.js Outdated
@paulirish paulirish mentioned this pull request Sep 19, 2018
3 tasks done

@paulirish paulirish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm % brendan's thing

Comment thread lighthouse-core/gather/driver.js Outdated

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

Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread lighthouse-core/gather/driver.js Outdated

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

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.

so. much. easier. :D

@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!

🐎🏎🚤🏃💨

@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