Skip to content

core(load-fast-4-pwa): use computed artifacts #4981

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.

Sign up for GitHub

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

Merged
patrickhulce merged 4 commits into from
Apr 18, 2018
Merged

core(load-fast-4-pwa): use computed artifacts #4981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9349c94
core(load-fast-4-pwa): use computed artifact
patrickhulce Apr 13, 2018
2f91312
update fixture
patrickhulce Apr 17, 2018
36c03e0
feedback
patrickhulce Apr 18, 2018
8093712
Merge branch 'master' into load_fast_4_pwa_lantern
patrickhulce Apr 18, 2018
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
148 changes: 38 additions & 110 deletions lighthouse-core/audits/load-fast-enough-for-pwa.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,14 @@
* Afterwards, we report if the TTFI is less than 10 seconds.
*/

const isDeepEqual = require('lodash.isequal');
const Audit = require('./audit');
const URL = require('../lib/url-shim');
const targetLatencyMs = require('../config/constants').throttling.mobile3G.rttMs;
const Sentry = require('../lib/sentry');
const mobile3GThrottling = require('../config/constants').throttling.mobile3G;
const Util = require('../report/v2/renderer/util.js');

// Maximum TTFI to be considered "fast" for PWA baseline checklist
// Maximum TTI to be considered "fast" for PWA baseline checklist
// https://developers.google.com/web/progressive-web-apps/checklist
const MAXIMUM_TTFI = 10 * 1000;

const WHITELISTED_STATUS_CODES = [307];
const MAXIMUM_TTI = 10 * 1000;
Copy link
Member

@paulirish paulirish Apr 18, 2018

Choose a reason for hiding this comment

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

Just to call out explicitly:
We're changing the PWA speed threshold from TTFI <= 10s to be a slightly more aggressive TTI < 10s.


class LoadFastEnough4Pwa extends Audit {
/**
Expand All @@ -32,116 +29,47 @@ class LoadFastEnough4Pwa extends Audit {
name: 'load-fast-enough-for-pwa',
description: 'Page load is fast enough on 3G',
failureDescription: 'Page load is not fast enough on 3G',
helpText: 'A fast page load over a 3G network ensures a good mobile user experience. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).',
helpText:
'A fast page load over a 3G network ensures a good mobile user experience. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).',
requiredArtifacts: ['traces', 'devtoolsLogs'],
};
}

/**
* @param {!Artifacts} artifacts
* @return {!AuditResult}
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {LH.AuditResult}
*/
static audit(artifacts) {
const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
const firstRequestLatenciesByOrigin = new Map();
networkRecords.forEach(record => {
// Ignore requests that don't have valid origin, timing data, came from the cache, were
// redirected by Chrome without going to the network, or are not finished.
const fromCache = record._fromDiskCache || record._fromMemoryCache;
const origin = URL.getOrigin(record._url);
if (!origin || !record._timing || fromCache ||
WHITELISTED_STATUS_CODES.includes(record.statusCode) || !record.finished) {
return;
}

// Disregard requests with an invalid start time, (H2 request start times are sometimes less
// than issue time and even negative which throws off timing)
if (record._startTime < record._issueTime) {
return;
}

// Use DevTools' definition of Waiting latency: https://github.com/ChromeDevTools/devtools-frontend/blob/66595b8a73a9c873ea7714205b828866630e9e82/front_end/network/RequestTimingView.js#L164
const latency = record._timing.receiveHeadersEnd - record._timing.sendEnd;
const latencyInfo = {
url: record._url,
startTime: record._startTime,
origin,
latency,
};

// Only examine the first request per origin to reduce noisiness from cases like H2 push
// where individual request latency may not apply.
const existing = firstRequestLatenciesByOrigin.get(origin);
if (!existing || latencyInfo.startTime < existing.startTime) {
firstRequestLatenciesByOrigin.set(origin, latencyInfo);
}
});

let firstRequestLatencies = Array.from(firstRequestLatenciesByOrigin.values());
const latency3gMin = targetLatencyMs - 10;
const areLatenciesAll3G = firstRequestLatencies.every(val => val.latency > latency3gMin);
firstRequestLatencies = firstRequestLatencies.map(item => ({
url: item.url,
latency: Util.formatNumber(item.latency, 0.01),
}));

const trace = artifacts.traces[Audit.DEFAULT_PASS];
return artifacts.requestFirstInteractive(trace).then(firstInteractive => {
const timeToFirstInteractive = firstInteractive.timeInMs;
const isFast = timeToFirstInteractive < MAXIMUM_TTFI;

const extendedInfo = {
value: {areLatenciesAll3G, firstRequestLatencies, isFast, timeToFirstInteractive},
};
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];

// If throttling was default devtools or lantern 3G throttling, then reuse the given settings
// Otherwise, we'll force the usage of lantern 3G.
const settingOverrides = {throttlingMethod: 'simulate', throttling: mobile3GThrottling};
const settings =
context.settings.throttlingMethod !== 'provided' &&
isDeepEqual(context.settings.throttling, mobile3GThrottling)
? context.settings
: Object.assign({}, context.settings, settingOverrides);

const metricComputationData = {trace, devtoolsLog, settings};
const tti = await artifacts.requestConsistentlyInteractive(metricComputationData);

const score = Number(tti.timing < MAXIMUM_TTI);

let debugString;
if (!score) {
// eslint-disable-next-line max-len
debugString = `First Interactive was ${Util.formatMilliseconds(tti.timing)}. More details in the "Performance" section.`;
}

const details = Audit.makeTableDetails([
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'latency', itemType: 'text', text: 'Latency (ms)'},
], firstRequestLatencies);

if (!isFast) {
return {
rawValue: false,
// eslint-disable-next-line max-len
debugString: `First Interactive was at ${Util.formatMilliseconds(timeToFirstInteractive)}. More details in the "Performance" section.`,
extendedInfo,
};
}

if (!areLatenciesAll3G) {
const sentryContext = Sentry.getContext();
const hadThrottlingEnabled = sentryContext && sentryContext.extra &&
sentryContext.extra.networkThrottling;

if (hadThrottlingEnabled) {
// Track these instances in Sentry since there should be no requests that are fast when
// throttling is enabled, and it's likely a throttling bug we should look into.
const violatingLatency = firstRequestLatencies
.find(item => Number(item.latency) < latency3gMin);
Sentry.captureMessage('Network request latencies were not realistic', {
tags: {audit: this.meta.name},
extra: {violatingLatency},
level: 'warning',
});
}

return {
rawValue: true,
// eslint-disable-next-line max-len
debugString: `First Interactive was found at ${Util.formatMilliseconds(timeToFirstInteractive)}; however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`,
extendedInfo,
details,
};
}

return {
rawValue: true,
extendedInfo,
};
});
});
return {
score,
debugString,
rawValue: tti.timing,
};
}
}

Expand Down
4 changes: 4 additions & 0 deletions lighthouse-core/gather/computed/metrics/metric.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ class ComputedMetric extends ComputedArtifact {
*/
async compute_(data, artifacts) {
const {trace, devtoolsLog, settings} = data;
if (!trace || !devtoolsLog || !settings) {
throw new Error('Did not provide necessary metric computation data');
}

const augmentedData = Object.assign({
networkRecords: await artifacts.requestNetworkRecords(devtoolsLog),
traceOfTab: await artifacts.requestTraceOfTab(trace),
Expand Down
88 changes: 33 additions & 55 deletions lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,88 +6,66 @@
'use strict';

const FastPWAAudit = require('../../audits/load-fast-enough-for-pwa');
const Runner = require('../../runner.js');
const Audit = require('../../audits/audit.js');
const mobile3GThrottling = require('../../config/constants').throttling.mobile3G;
const assert = require('assert');

function generateArtifacts(firstInteractiveValue, networkRecords = []) {
const trace = require('../fixtures/traces/progressive-app-m60.json');
const devtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json');

function generateArtifacts(ttiValue) {
return {
devtoolsLogs: {
[Audit.DEFAULT_PASS]: [],
},
requestNetworkRecords: () => {
return Promise.resolve(networkRecords);
},
traces: {
[Audit.DEFAULT_PASS]: {traceEvents: []},
},
requestFirstInteractive: () => Promise.resolve({
timeInMs: firstInteractiveValue,
requestConsistentlyInteractive: () => Promise.resolve({
timing: ttiValue,
}),
};
}

/* eslint-env mocha */
describe('PWA: load-fast-enough-for-pwa audit', () => {
it('returns boolean based on TTI value', () => {
return FastPWAAudit.audit(generateArtifacts(5000)).then(result => {
assert.equal(result.rawValue, true, 'fixture trace is not passing audit');
const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
return FastPWAAudit.audit(generateArtifacts(5000), {settings}).then(result => {
assert.equal(result.score, true, 'fixture trace is not passing audit');
assert.equal(result.rawValue, 5000);
});
});

it('fails a bad TTI value', () => {
return FastPWAAudit.audit(generateArtifacts(15000)).then(result => {
assert.equal(result.rawValue, false, 'not failing a long TTI value');
const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
return FastPWAAudit.audit(generateArtifacts(15000), {settings}).then(result => {
assert.equal(result.score, false, 'not failing a long TTI value');
assert.equal(result.rawValue, 15000);
assert.ok(result.debugString);
});
});

it('warns on a good TTI value with no throttling', () => {
// latencies are very short
const mockNetworkRecords = [
{_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/'},
{_timing: {sendEnd: 0, receiveHeadersEnd: 75}, finished: true, _url: 'https://google.com/a'},
{ },
{_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/b'},
];
return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
assert.equal(result.rawValue, true);
assert.ok(result.debugString.includes('network request latencies'));
assert.ok(result.details, 'contains details when latencies were not realistic');
});
});
it('respects the observed result when throttling is preset', async () => {
const artifacts = Object.assign({
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: devtoolsLog},
}, Runner.instantiateComputedArtifacts());

it('ignores resources coming from cache', () => {
const mockNetworkRecords = [
{_timing: {sendEnd: 0, receiveHeadersEnd: 50}, _fromDiskCache: true},
];
return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
assert.equal(result.rawValue, true);
assert.strictEqual(result.debugString, undefined);
});
const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
const result = await FastPWAAudit.audit(artifacts, {settings});
assert.equal(Math.round(result.rawValue), 1582);
});

it('passes a good TTI value and WITH throttling', () => {
// latencies are very long
const urlA = 'https://google.com';
const urlB = 'https://example.com';
const urlC = 'https://example-c.com';
const mockNetworkRecords = [
{_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlA, _startTime: 0},
{_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlB},
// ignored for not having timing
{ },
// ignored for not being the first of the origin
{_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlA, _startTime: 100},
// ignored for being redirected internally
{_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlC, _startTime: 0,
statusCode: 307},
// ignored for not finishing
{_timing: {sendEnd: 0, receiveHeadersEnd: -1}, finished: false},
];
return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
assert.equal(result.rawValue, true);
assert.strictEqual(result.debugString, undefined);
assert.ok(!result.details, 'does not contain details when latencies are realistic');
});
it('overrides with simulated result when throttling is modified', async () => {
const artifacts = Object.assign({
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: devtoolsLog},
}, Runner.instantiateComputedArtifacts());

const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
const result = await FastPWAAudit.audit(artifacts, {settings});
assert.equal(Math.round(result.rawValue), 5308);
});
});
21 changes: 2 additions & 19 deletions lighthouse-core/test/results/sample_v2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,7 @@
"load-fast-enough-for-pwa": {
"score": 1,
"displayValue": "",
"rawValue": true,
"extendedInfo": {
"value": {
"areLatenciesAll3G": true,
"firstRequestLatencies": [
{
"url": "http://localhost:10200/dobetterweb/dbw_tester.html",
"latency": "570.56"
},
{
"url": "http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js",
"latency": "564.12"
}
],
"isFast": true,
"timeToFirstInteractive": 4927.278
}
},
"rawValue": 4927.278,
"scoreDisplayMode": "binary",
"name": "load-fast-enough-for-pwa",
"description": "Page load is fast enough on 3G",
Expand Down Expand Up @@ -5403,6 +5386,6 @@
}
},
"timing": {
"total": 877
"total": 788
}
}