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

Measure and visualize perf metrics #1936

Merged
merged 3 commits into from
Apr 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ lighthouse-cli/types/*.js
# Handlebar-templates
lighthouse-core/report/templates/report-templates.js
lighthouse-core/report/partials/templates/report-partials.js

# Generated files
plots/out/**
Copy link
Member

Choose a reason for hiding this comment

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

should add this to .npmignore as well. Also, do we want to ship plots with the npm module? Could be useful, but we might want to wait on that, so we could npmignore plots/ entirely for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's npmignore plots entirely for now. Once it's been more thoroughly tested and documented, then it makes sense to ship it with the npm module.

Copy link
Member

Choose a reason for hiding this comment

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

Let's npmignore plots entirely for now

SGTM

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ lighthouse-cli/types/*.map

lighthouse-core/report/partials/templates/
lighthouse-core/report/templates/*.js

plots/out/
2 changes: 2 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ lighthouse-cli/types/*.map
node_modules/
results/

plots/

# generated files
**/pages/scripts/lighthouse-report.js
*.report.html
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/audits/time-to-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ class TTIMetric extends Audit {
throw new Error('No firstMeaningfulPaint event found in trace');
}

const onLoadTiming = tabTrace.timings.onLoad;
const fmpTiming = tabTrace.timings.firstMeaningfulPaint;
const traceEndTiming = tabTrace.timings.traceEnd;

Expand Down Expand Up @@ -214,6 +215,7 @@ class TTIMetric extends Audit {

const extendedInfo = {
timings: {
onLoad: onLoadTiming,
fMP: parseFloat(fmpTiming.toFixed(3)),
visuallyReady: parseFloat(visuallyReadyTiming.toFixed(3)),
timeToInteractive: parseFloat(timeToInteractive.timeInMs.toFixed(3)),
Expand All @@ -222,6 +224,7 @@ class TTIMetric extends Audit {
endOfTrace: traceEndTiming,
},
timestamps: {
onLoad: (onLoadTiming + navStartTsInMS) * 1000,
fMP: fMPtsInMS * 1000,
visuallyReady: (visuallyReadyTiming + navStartTsInMS) * 1000,
timeToInteractive: (timeToInteractive.timeInMs + navStartTsInMS) * 1000,
Expand Down
16 changes: 16 additions & 0 deletions lighthouse-core/config/plots.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"passes": [{
"recordNetwork": true,
"recordTrace": true,
"pauseBeforeTraceEndMs": 5000,
"useThrottling": true,
"gatherers": []
}],

"audits": [
"first-meaningful-paint",
"speed-index-metric",
"estimated-input-latency",
"time-to-interactive"
]
}
5 changes: 5 additions & 0 deletions lighthouse-core/gather/computed/trace-of-tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class TraceOfTab extends ComputedArtifact {
.filter(e => {
return e.cat.includes('blink.user_timing') ||
e.cat.includes('loading') ||
e.cat.includes('devtools.timeline') ||
e.name === 'TracingStartedInPage';
})
.sort((event0, event1) => event0.ts - event1.ts);
Expand Down Expand Up @@ -87,6 +88,8 @@ class TraceOfTab extends ComputedArtifact {
firstMeaningfulPaint = lastCandidate;
}

const onLoad = keyEvents.find(e => e.name === 'MarkLoad' && e.ts > navigationStart.ts);

// subset all trace events to just our tab's process (incl threads other than main)
const processEvents = trace.traceEvents
.filter(e => e.pid === startedInPageEvt.pid)
Expand All @@ -102,6 +105,7 @@ class TraceOfTab extends ComputedArtifact {
firstContentfulPaint,
firstMeaningfulPaint,
traceEnd,
onLoad,
};

const timings = {};
Expand All @@ -121,6 +125,7 @@ class TraceOfTab extends ComputedArtifact {
firstPaintEvt: firstPaint,
firstContentfulPaintEvt: firstContentfulPaint,
firstMeaningfulPaintEvt: firstMeaningfulPaint,
onLoadEvt: onLoad,
};
}
}
Expand Down
100 changes: 90 additions & 10 deletions lighthouse-core/lib/traces/pwmetrics-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@

const log = require('../../../lighthouse-core/lib/log.js');

/**
* @param {!Object} object
* @param {string} path
* @return {*}
*/
function safeGet(object, path) {
Copy link
Member

Choose a reason for hiding this comment

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

not wild about this, mostly because it completely disables typing. Is the issue that the intermediate objects are sometimes undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right - this happens when the audit fails. An alternative is to always to have the intermediate objects be created even when the audit fails.

Copy link
Member

Choose a reason for hiding this comment

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

An alternative is to always to have the intermediate objects be created even when the audit fails.

I think it's reasonable not to have an extendedInfo if the audit failed, but if it didn't fail, all the extendedInfo paths are filled or explicitly null (or whatever) at the leaf properties.

I'm sighing via code review because I'm currently in the process of re-enabling closure compiler for the codebase and this will all have to be undone again :)

I don't want to block plots/ on having to hunt down all the current failure modes though, so we can go with this for now and file an issue about extendedInfo structure guarantees (which having a type check test will help with :)

const components = path.split('.');
for (const component of components) {
if (!object) {
return null;
}
object = object[component];
}
return object;
}

class Metrics {
constructor(traceEvents, auditResults) {
this._traceEvents = traceEvents;
Expand All @@ -35,81 +51,145 @@ class Metrics {
id: 'navstart',
getTs: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return fmpExt.value.timestamps.navStart;
return safeGet(fmpExt, 'value.timestamps.navStart');
},
getTiming: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return safeGet(fmpExt, 'value.timings.navStart');
}
},
{
name: 'First Contentful Paint',
id: 'ttfcp',
getTs: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return fmpExt.value.timestamps.fCP;
return safeGet(fmpExt, 'value.timestamps.fCP');
},
getTiming: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return safeGet(fmpExt, 'value.timings.fCP');
}
},
{
name: 'First Meaningful Paint',
id: 'ttfmp',
getTs: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return fmpExt.value.timestamps.fMP;
return safeGet(fmpExt, 'value.timestamps.fMP');
},
getTiming: auditResults => {
const fmpExt = auditResults['first-meaningful-paint'].extendedInfo;
return safeGet(fmpExt, 'value.timings.fMP');
}
},
{
name: 'Perceptual Speed Index',
id: 'psi',
getTs: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return siExt.value.timestamps.perceptualSpeedIndex;
return safeGet(siExt, 'value.timestamps.perceptualSpeedIndex');
},
getTiming: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return safeGet(siExt, 'value.timings.perceptualSpeedIndex');
}
},
{
name: 'First Visual Change',
id: 'fv',
getTs: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return siExt.value.timestamps.firstVisualChange;
return safeGet(siExt, 'value.timestamps.firstVisualChange');
},
getTiming: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return safeGet(siExt, 'value.timings.firstVisualChange');
}
},
{
name: 'Visually Complete 85%',
id: 'vc85',
getTs: auditResults => {
const siExt = auditResults['time-to-interactive'].extendedInfo;
return siExt.value.timestamps.visuallyReady;
return safeGet(siExt, 'value.timestamps.visuallyReady');
},
getTiming: auditResults => {
const siExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(siExt, 'value.timings.visuallyReady');
}
},
{
name: 'Visually Complete 100%',
id: 'vc100',
getTs: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return siExt.value.timestamps.visuallyComplete;
return safeGet(siExt, 'value.timestamps.visuallyComplete');
},
getTiming: auditResults => {
const siExt = auditResults['speed-index-metric'].extendedInfo;
return safeGet(siExt, 'value.timings.visuallyComplete');
}
},
{
name: 'Time to Interactive (vAlpha)',
id: 'tti',
getTs: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return ttiExt.value.timestamps.timeToInteractive;
return safeGet(ttiExt, 'value.timestamps.timeToInteractive');
},
getTiming: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timings.timeToInteractive');
}
},
{
name: 'Time to Interactive (vAlpha non-visual)',
id: 'tti-non-visual',
getTs: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return ttiExt.value.timestamps.timeToInteractiveB;
return safeGet(ttiExt, 'value.timestamps.timeToInteractiveB');
},
getTiming: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timings.timeToInteractiveB');
}
},
{
name: 'Time to Interactive (vAlpha non-visual, 5s)',
id: 'tti-non-visual-5s',
getTs: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return ttiExt.value.timestamps.timeToInteractiveC;
return safeGet(ttiExt, 'value.timestamps.timeToInteractiveC');
},
getTiming: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timings.timeToInteractiveC');
}
},
{
name: 'End of Trace',
id: 'eot',
getTs: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timestamps.endOfTrace');
},
getTiming: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timings.endOfTrace');
}
},
{
name: 'On Load',
id: 'onload',
getTs: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timestamps.onLoad');
},
getTiming: auditResults => {
const ttiExt = auditResults['time-to-interactive'].extendedInfo;
return safeGet(ttiExt, 'value.timings.onLoad');
}
}
];
}

Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/test/fixtures/dbw_tester-perf-results.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
"extendedInfo": {
"value": {
"timings": {
"onLoad": 2956.185,
"fMP": 2946.14,
"visuallyReady": 2955.548,
"timeToInteractive": 2955.548,
Expand All @@ -121,6 +122,7 @@
"endOfTrace": 8471.488999843597
},
"timestamps": {
"onLoad": 1082005348861.9999,
"fMP": 1082005338816.9999,
"visuallyReady": 1082005348224.9999,
"timeToInteractive": 1082005348224.9999,
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/test/lib/traces/pwmetrics-events-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ const assert = require('assert');
const dbwTrace = require('../../fixtures/traces/dbw_tester.json');
const dbwResults = require('../../fixtures/dbw_tester-perf-results.json');


/* eslint-env mocha */
describe('metrics events class', () => {
it('exposes metric definitions', () => {
assert.equal(Metrics.metricsDefinitions.length, 10, '10 metrics not exposed');
assert.equal(Metrics.metricsDefinitions.length, 12, '12 metrics not exposed');
});

it('generates fake trace events', () => {
Expand Down
27 changes: 27 additions & 0 deletions plots/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Lighthouse Metrics Analysis

For context and roadmap, please see issue:
https://github.com/GoogleChrome/lighthouse/issues/1924

## Workflow

### Setup

You need to build lighthouse first.

### Commands

```
# View all commands
$ cd plots
$ npm run

# Run lighthouse to collect metrics data
$ npm run measure

# Analyze the data to generate a summary file (i.e. out/generatedResults.js)
$ npm run analyze

# View visualization
# Open index.html in browser
```
Loading