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

add lighthouse calculation time to json results #2241

Merged
merged 2 commits into from
May 13, 2017
Merged

Conversation

brendankenny
Copy link
Member

happy to bikeshed on the name and where it should be taken, but basically it would be nice to be able to track how long a lighthouse run takes. I was going to do it in plots/, but @wwwillchen pointed out that it's actually a generally useful thing to have in the LHR.

We could put this inside runner, but it's actually kind of nice to track config and connection construction too. This does ignore work done in bin.ts, however.

@@ -38,9 +38,10 @@ const Config = require('./config/config');
*/

module.exports = function(url, flags = {}, configJSON) {
return new Promise((resolve, reject) => {
const startTime = Date.now();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Audit error: Avoids Date.now() In Its Own Scripts ;)

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

I'd probably prefer something like totalRunTime since I can seecalculationTime being reserved for post-gather phase or something as we want to get more granular, but I'm cool with this too

@brendankenny
Copy link
Member Author

no, that was my best effort at "timeDoingStuff" :) totalRunTime SGTM

@brendankenny
Copy link
Member Author

brendankenny commented May 12, 2017

I guess the only confusing thing is the top-level runtimeConfig...runtime vs runTime :)

@paulirish
Copy link
Member

paulirish commented May 12, 2017

how about a timing object:

LHR.timing.total // 43752 (ms)

(i imagine we'll want breakdown by pass, gather audit later on)

name bikeshed: runTiming also wfm

@patrickhulce
Copy link
Collaborator

how about a timing object:

love timing no need to add run prefixes IMO 👍

@brendankenny
Copy link
Member Author

timing sounds great

@brendankenny
Copy link
Member Author

I'm adding this to plots/, just sec :)

@brendankenny
Copy link
Member Author

plots/ now graphs the total execution time for runs (and can expand as we add other timings)

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

3 participants