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

core(legacy-javascript): make opportunity #10881

Merged
merged 42 commits into from Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0f8f368
first pass
connorjclark May 29, 2020
d03031d
graph
connorjclark May 30, 2020
c4f8565
update
connorjclark May 30, 2020
b719e69
deletecode
connorjclark May 30, 2020
d175a5b
updaet
connorjclark May 30, 2020
7dbf207
pr
connorjclark Jun 3, 2020
27ff8e1
pr
connorjclark Jun 3, 2020
2ba238d
fix leg js test
connorjclark Jun 8, 2020
df85c75
wasted bytes
connorjclark Jun 8, 2020
7ce71f2
Update lighthouse-core/scripts/legacy-javascript/create-polyfill-size…
connorjclark Jun 8, 2020
fef1d71
lint
connorjclark Jun 8, 2020
8c97699
update
connorjclark Jun 8, 2020
62f970a
estimate transform
connorjclark Jun 8, 2020
436f302
est
connorjclark Jun 8, 2020
c5de8fc
minor
connorjclark Jun 8, 2020
9352cb5
Merge remote-tracking branch 'origin/master' into leg-js-opp
connorjclark Jun 8, 2020
ee78778
fix
connorjclark Jun 8, 2020
966a8cf
types
connorjclark Jun 9, 2020
c2557d4
minor
connorjclark Jun 9, 2020
5b88ce0
fix smoke
connorjclark Jun 9, 2020
fef8ab6
update
connorjclark Jun 9, 2020
799742b
script
connorjclark Jun 9, 2020
01d4c1b
Merge remote-tracking branch 'origin/master' into leg-js-opp
connorjclark Jun 16, 2020
c171406
update
connorjclark Jun 16, 2020
4718690
fix test
connorjclark Jun 16, 2020
c3f78e3
comment
connorjclark Jun 16, 2020
d2308d5
gran
connorjclark Jun 16, 2020
d57bf16
edit
connorjclark Jun 16, 2020
ded30d5
Merge remote-tracking branch 'origin/master' into leg-js-opp
connorjclark Jun 19, 2020
833da8d
minor
connorjclark Jun 19, 2020
b6c690a
maybe fix merge
connorjclark Jun 19, 2020
bfed5cf
undo duplicated js changes
connorjclark Jun 19, 2020
51a55a4
refactor transfer ratio
connorjclark Jun 19, 2020
962d423
use estimateTransferRatio in duplicated js audit too
connorjclark Jun 19, 2020
4389b41
lint
connorjclark Jun 19, 2020
6c1bde6
lint
connorjclark Jun 20, 2020
e0d5d95
Update lighthouse-core/audits/byte-efficiency/legacy-javascript.js
connorjclark Jun 23, 2020
3aa2fe0
comment
connorjclark Jun 23, 2020
3931a5f
estimateTransferRatioForScript just for LegacyJavascript
connorjclark Jun 23, 2020
52da233
lint
connorjclark Jun 24, 2020
db2772c
revrt
connorjclark Jun 24, 2020
dd5b669
Merge remote-tracking branch 'origin/master' into leg-js-opp
connorjclark Jul 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 35 additions & 3 deletions lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
Expand Up @@ -9,6 +9,7 @@ const Audit = require('../audit.js');
const linearInterpolation = require('../../lib/statistics.js').linearInterpolation;
const Interactive = require('../../computed/metrics/lantern-interactive.js');
const i18n = require('../../lib/i18n/i18n.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const NetworkRecords = require('../../computed/network-records.js');
const LoadSimulator = require('../../computed/load-simulator.js');
const PageDependencyGraph = require('../../computed/page-dependency-graph.js');
Expand Down Expand Up @@ -38,7 +39,7 @@ const WASTED_MS_FOR_SCORE_OF_ZERO = 5000;
* @overview Used as the base for all byte efficiency audits. Computes total bytes
* and estimated time saved. Subclass and override `audit_` to return results.
*/
class UnusedBytes extends Audit {
class ByteEfficiencyAudit extends Audit {
/**
* Creates a score based on the wastedMs value using linear interpolation between control points.
*
Expand Down Expand Up @@ -216,7 +217,7 @@ class UnusedBytes extends Audit {
displayValue,
numericValue: wastedMs,
numericUnit: 'millisecond',
score: UnusedBytes.scoreForWastedMs(wastedMs),
score: ByteEfficiencyAudit.scoreForWastedMs(wastedMs),
extendedInfo: {
value: {
wastedMs,
Expand All @@ -228,6 +229,37 @@ class UnusedBytes extends Audit {
};
}

/**
* Convert bytes to transfer size estimation.
* @param {LH.Artifacts} artifacts
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
* @param {Map<string, number>} wastedBytesByUrl
*/
static async convertWastedResourceBytesToTransferBytes(artifacts, networkRecords, wastedBytesByUrl) {
const mainDocumentRecord = await NetworkAnalyzer.findMainDocument(networkRecords);
for (const [url, bytes] of wastedBytesByUrl.entries()) {
const networkRecord = url === artifacts.URL.finalUrl ?
mainDocumentRecord :
networkRecords.find(n => n.url === url);
const script = artifacts.ScriptElements.find(script => script.src === url);
if (!script || script.content === null) {
// This should never happen because we found the wasted bytes from bundles, which required contents in a ScriptElement.
continue;
}
if (!networkRecord) {
// This should never happen because we either have a network request for the main document (inline scripts),
// or the ScriptElement if for an external resource and so should have a network request.
continue;
}

const contentLength = script.content.length;
const transferSize =
ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength, 'Script');
const transferRatio = transferSize / contentLength;
wastedBytesByUrl.set(url, bytes * transferRatio);
}
}

/* eslint-disable no-unused-vars */

/**
Expand All @@ -243,4 +275,4 @@ class UnusedBytes extends Audit {
/* eslint-enable no-unused-vars */
}

module.exports = UnusedBytes;
module.exports = ByteEfficiencyAudit;
25 changes: 1 addition & 24 deletions lighthouse-core/audits/byte-efficiency/duplicated-javascript.js
Expand Up @@ -7,7 +7,6 @@

const ByteEfficiencyAudit = require('./byte-efficiency-audit.js');
const ModuleDuplication = require('../../computed/module-duplication.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
Expand Down Expand Up @@ -187,29 +186,7 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
items.push(otherItem);
}

// Convert bytes to transfer size estimation.
const mainDocumentRecord = await NetworkAnalyzer.findMainDocument(networkRecords);
for (const [url, bytes] of wastedBytesByUrl.entries()) {
const networkRecord = url === artifacts.URL.finalUrl ?
mainDocumentRecord :
networkRecords.find(n => n.url === url);
const script = artifacts.ScriptElements.find(script => script.src === url);
if (!script || script.content === null) {
// This should never happen because we found the wasted bytes from bundles, which required contents in a ScriptElement.
continue;
}
if (!networkRecord) {
// This should never happen because we either have a network request for the main document (inline scripts),
// or the ScriptElement if for an external resource and so should have a network request.
continue;
}

const contentLength = script.content.length;
const transferSize =
ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength, 'Script');
const transferRatio = transferSize / contentLength;
wastedBytesByUrl.set(url, bytes * transferRatio);
}
await this.convertWastedResourceBytesToTransferBytes(artifacts, networkRecords, wastedBytesByUrl);

/** @type {LH.Audit.Details.OpportunityColumnHeading[]} */
const headings = [
Expand Down
Expand Up @@ -15,11 +15,10 @@
/** @typedef {{name: string, expression: string}} Pattern */
/** @typedef {{name: string, line: number, column: number}} PatternMatchResult */

const Audit = require('./audit.js');
const NetworkRecords = require('../computed/network-records.js');
const JSBundles = require('../computed/js-bundles.js');
const i18n = require('../lib/i18n/i18n.js');
const thirdPartyWeb = require('../lib/third-party-web.js');
const ByteEfficiencyAudit = require('./byte-efficiency-audit.js');
const JsBundles = require('../../computed/js-bundles.js');
const i18n = require('../../lib/i18n/i18n.js');
const thirdPartyWeb = require('../../lib/third-party-web.js');

const UIStrings = {
/** Title of a Lighthouse audit that tells the user about legacy polyfills and transforms used on the page. This is displayed in a list of audit titles that Lighthouse generates. */
Expand Down Expand Up @@ -97,17 +96,17 @@ class CodePatternMatcher {
}
}

class LegacyJavascript extends Audit {
class LegacyJavascript extends ByteEfficiencyAudit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'legacy-javascript',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC,
description: str_(UIStrings.description),
title: str_(UIStrings.title),
requiredArtifacts: ['devtoolsLogs', 'ScriptElements', 'SourceMaps', 'URL'],
requiredArtifacts: ['devtoolsLogs', 'traces', 'ScriptElements', 'SourceMaps', 'URL'],
};
}

Expand Down Expand Up @@ -348,18 +347,89 @@ class LegacyJavascript extends Audit {
return urlToMatchResults;
}

/**
* @param {PatternMatchResult[]} matches
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
*/
static estimateWastedBytes(matches) {
const polyfillSignals = matches.filter(m => !m.name.startsWith('@')).map(m => m.name);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const transformSignals = matches.filter(m => m.name.startsWith('@')).map(m => m.name);

// experimenting.
const POLYFILL_USE_GRAPH = true;

let estimatedWastedBytesFromPolyfills = 0;
if (POLYFILL_USE_GRAPH) {
// TODO
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
const graph = require('./polyfill-graph-data.json');
const modulesSeen = new Set();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const modulesSeen = new Set();
const modulesIndicesSeen = new Set();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

counter proposal: just use moduleIndex in the reduce below. doesn't matter for the set operations to distinguish indices from string here.

for (const polyfillSignal of polyfillSignals) {
// @ts-ignore: lacking index type.
const modules = graph.dependencies[polyfillSignal];
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
for (const module of modules) {
modulesSeen.add(module);
}
}

if (polyfillSignals.length > 0) estimatedWastedBytesFromPolyfills += graph.baseSize;
estimatedWastedBytesFromPolyfills += graph.baseSize;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
estimatedWastedBytesFromPolyfills += [...modulesSeen].reduce((acc, module) => {
return acc + graph.moduleSizes[module];
}, 0);
// Not needed?
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah maybe just add a test for this rather than an extra min?

estimatedWastedBytesFromPolyfills =
Math.min(estimatedWastedBytesFromPolyfills, graph.maxSize);
} else {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const typedArrayPolyfills = [
'Float32Array',
'Float64Array',
'Int16Array',
'Int32Array',
'Int8Array',
'Uint16Array',
'Uint32Array',
'Uint8Array',
'Uint8ClampedArray',
];
const typedArrayCount = typedArrayPolyfills.filter(p => polyfillSignals.includes(p)).length;

const POLYFILL_MAX_WASTE = 175764;
const POLYFILL_TYPED_WASTE = 40 * 1000; // ~60KiB, minus the base 20.
const POLYFILL_BASE_SIZE = 20 * 1000;

if (typedArrayCount > 0) estimatedWastedBytesFromPolyfills += POLYFILL_TYPED_WASTE;
if (polyfillSignals.length > 0) estimatedWastedBytesFromPolyfills += POLYFILL_BASE_SIZE;
// Linear estimation of non-typed polyfills (and minus one to exclude the base size above).
estimatedWastedBytesFromPolyfills += (polyfillSignals.length - typedArrayCount - 1)
* (POLYFILL_MAX_WASTE - POLYFILL_TYPED_WASTE - POLYFILL_BASE_SIZE)
/ (this.getPolyfillData().length - typedArrayPolyfills.length - 1);
estimatedWastedBytesFromPolyfills =
Math.min(estimatedWastedBytesFromPolyfills, POLYFILL_MAX_WASTE);
}

const estimatedWastedBytesFromTransforms = 0;
// TODO

const estimatedWastedBytes =
Copy link
Collaborator

Choose a reason for hiding this comment

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

love the explicitness here 👍

estimatedWastedBytesFromPolyfills + estimatedWastedBytesFromTransforms;
return estimatedWastedBytes;
}

/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
* @return {Promise<ByteEfficiencyAudit.ByteEfficiencyProduct>}
*/
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[LegacyJavascript.DEFAULT_PASS];
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const bundles = await JSBundles.request(artifacts, context);
static async audit_(artifacts, networkRecords, context) {
const mainDocumentEntity = thirdPartyWeb.getEntity(artifacts.URL.finalUrl);
const bundles = await JsBundles.request(artifacts, context);

/**
* @typedef {LH.Audit.ByteEfficiencyItem & {signals: string[], locations: LH.Audit.Details.SourceLocationValue[]}} Item
*/

/** @type {Array<{url: string, signals: string[], locations: LH.Audit.Details.SourceLocationValue[]}>} */
const tableRows = [];
/** @type {Item[]} */
const items = [];
let signalCount = 0;

// TODO(cjamcl): Use SourceMaps, and only pattern match if maps are not available.
Expand All @@ -371,8 +441,21 @@ class LegacyJavascript extends Audit {
const urlToMatchResults =
this.detectAcrossScripts(matcher, artifacts.ScriptElements, networkRecords, bundles);
urlToMatchResults.forEach((matches, url) => {
/** @type {typeof tableRows[number]} */
const row = {url, signals: [], locations: []};
// Only estimate savings if first party code has legacy code.
let wastedBytes = 0;
if (thirdPartyWeb.isFirstParty(url, mainDocumentEntity)) {
wastedBytes = this.estimateWastedBytes(matches);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
}

/** @type {typeof items[number]} */
const row = {
url,
signals: [],
locations: [],
wastedBytes,
// Not needed, but keeps typescript happy.
totalBytes: 0,
};
for (const match of matches) {
const {name, line, column} = match;
row.signals.push(name);
Expand All @@ -384,31 +467,32 @@ class LegacyJavascript extends Audit {
urlProvider: 'network',
});
}
tableRows.push(row);
items.push(row);
signalCount += row.signals.length;
});

/** @type {LH.Audit.Details.Table['headings']} */
// /** @type {Map<string, number>} */
// const wastedBytesByUrl = new Map();
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
// for (const row of items) {
// // Only estimate savings if first party code has legacy code.
// if (!thirdPartyWeb.isFirstParty(row.url, mainDocumentEntity)) {
// continue;
// }
// }
// await this.convertWastedResourceBytesToTransferBytes(artifacts, networkRecords, wastedBytesByUrl);

/** @type {LH.Audit.Details.OpportunityColumnHeading[]} */
const headings = [
/* eslint-disable max-len */
{key: 'url', itemType: 'url', subRows: {key: 'locations', itemType: 'source-location'}, text: str_(i18n.UIStrings.columnURL)},
{key: null, itemType: 'code', subRows: {key: 'signals'}, text: ''},
{key: 'url', valueType: 'url', subRows: {key: 'locations', valueType: 'source-location'}, label: str_(i18n.UIStrings.columnURL)},
{key: null, valueType: 'code', subRows: {key: 'signals'}, label: ''},
{key: 'wastedBytes', valueType: 'bytes', granularity: 0.05, label: str_(i18n.UIStrings.columnWastedBytes)},
Copy link
Collaborator

Choose a reason for hiding this comment

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

love the additional actionability of this 👍

/* eslint-enable max-len */
];
const details = Audit.makeTableDetails(headings, tableRows);

// Only fail if first party code has legacy code.
const mainDocumentEntity = thirdPartyWeb.getEntity(artifacts.URL.finalUrl);
const foundSignalInFirstPartyCode = tableRows.some(row => {
return thirdPartyWeb.isFirstParty(row.url, mainDocumentEntity);
});
return {
score: foundSignalInFirstPartyCode ? 0 : 1,
notApplicable: !foundSignalInFirstPartyCode,
extendedInfo: {
signalCount,
},
details,
items,
headings,
};
}
}
Expand Down