Skip to content

Commit

Permalink
fix: always use navStart as speedline timeOrigin (#2114)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and paulirish committed May 1, 2017
1 parent ae0e375 commit 0549cca
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 24 deletions.
11 changes: 6 additions & 5 deletions lighthouse-core/gather/computed/speedline.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ class Speedline extends ComputedArtifact {
/**
* @return {!Promise}
*/
compute_(trace) {
compute_(trace, computedArtifacts) {
// speedline() may throw without a promise, so we resolve immediately
// to get in a promise chain.
return Promise.resolve().then(_ => {
return speedline(trace.traceEvents);
}).then(speedlineResults => {
return speedlineResults;
return computedArtifacts.requestTraceOfTab(trace).then(traceOfTab => {
// Force use of nav start as reference point for speedline
// See https://github.com/GoogleChrome/lighthouse/issues/2095
const navStart = traceOfTab.timestamps.navigationStart * 1000;
return speedline(trace.traceEvents, {timeOrigin: navStart});
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,36 @@
"tdur": 186,
"tts": 4516530
},
{
"pid": 93449,
"tid": 1295,
"ts": 900000000000,
"ph": "X",
"cat": "toplevel",
"name": "TracingStartedInPage",
"args": {
"data": {
"page": "dummy"
}
},
"dur": 189,
"tdur": 186,
"tts": 4516530
},
{
"pid": 93449,
"tid": 1295,
"ts": 900000000000,
"ph": "X",
"cat": "loading",
"name": "navigationStart",
"args": {
"frame": "dummy"
},
"dur": 189,
"tdur": 186,
"tts": 4516530
},
{
"pid": 93267,
"tid": 1295,
Expand Down
37 changes: 18 additions & 19 deletions lighthouse-core/test/gather/computed/speedline-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@

/* eslint-env mocha */

const SpeedlineGather = require('../../../gather/computed/speedline.js');
const assert = require('assert');
const pwaTrace = require('../../fixtures/traces/progressive-app.json');
const threeFrameTrace = require('../../fixtures/traces/threeframes-blank_content_more.json');
const Runner = require('../../../runner.js');

describe('Speedline gatherer', () => {
it('returns an error debugString on faulty trace data', () => {
const speedlineGather = new SpeedlineGather();
let computedArtifacts;

beforeEach(() => {
computedArtifacts = Runner.instantiateComputedArtifacts();
});

return speedlineGather.request({traceEvents: {boo: 'ya'}}).then(_ => {
it('returns an error debugString on faulty trace data', () => {
return computedArtifacts.requestSpeedline({traceEvents: {boo: 'ya'}}).then(_ => {
assert.fail(true, true, 'Invalid trace did not throw exception in speedline');
}, err => {
assert.ok(err);
Expand All @@ -35,41 +39,36 @@ describe('Speedline gatherer', () => {
});

it('measures the pwa.rocks example with speed index of 577', () => {
const speedlineGather = new SpeedlineGather();

return speedlineGather.request({traceEvents: pwaTrace}).then(speedline => {
return assert.equal(Math.floor(speedline.speedIndex), 577);
return computedArtifacts.requestSpeedline({traceEvents: pwaTrace}).then(speedline => {
return assert.equal(Math.floor(speedline.speedIndex), 561);
});
});

it('measures SI of 3 frame trace (blank @1s, content @2s, more content @3s)', () => {
const speedlineGather = new SpeedlineGather();

return speedlineGather.request({traceEvents: threeFrameTrace}).then(speedline => {
return computedArtifacts.requestSpeedline(threeFrameTrace).then(speedline => {
assert.equal(Math.floor(speedline.speedIndex), 2040);
return assert.equal(Math.floor(speedline.perceptualSpeedIndex), 2030);
});
});

it('uses a cache', () => {
const speedlineGather = new SpeedlineGather();
let start;
let firstResult;
const trace = {traceEvents: pwaTrace};
// repeat with the same input data twice
return Promise.resolve()
.then(_ => speedlineGather.request(trace))
.then(_ => {
.then(_ => computedArtifacts.requestSpeedline(trace))
.then(result => {
start = Date.now();
firstResult = result;
})
.then(_ => speedlineGather.request(trace))
.then(_ => computedArtifacts.requestSpeedline(trace))
.then(speedline => {
// on a MacBook Air, one run is 1000-1500ms
assert.ok(Date.now() - start < 50, 'Quick results come from the cache');
assert.equal(firstResult, speedline, 'Cache match matches');

assert.ok(speedlineGather._cache.has(trace), 'Cache reports a match');
assert.equal(speedlineGather._cache.get(trace), speedline, 'Cache match matches');

return assert.equal(Math.floor(speedline.speedIndex), 577);
return assert.equal(Math.floor(speedline.speedIndex), 561);
});
});
});

0 comments on commit 0549cca

Please sign in to comment.