diff --git a/cli/test/smokehouse/test-definitions/byte-efficiency.js b/cli/test/smokehouse/test-definitions/byte-efficiency.js index 01f057bbd551..641005ea4061 100644 --- a/cli/test/smokehouse/test-definitions/byte-efficiency.js +++ b/cli/test/smokehouse/test-definitions/byte-efficiency.js @@ -243,7 +243,7 @@ const expectations = { wastedBytes: '5827 +/- 200', subItems: { items: [ - {source: '…./b.js', sourceBytes: '4417 +/- 50', sourceWastedBytes: '2191 +/- 50'}, + {source: '…./b.js', sourceBytes: '4347 +/- 50', sourceWastedBytes: '2156 +/- 50'}, {source: '…./c.js', sourceBytes: '2200 +/- 50', sourceWastedBytes: '2182 +/- 50'}, {source: '…webpack/bootstrap', sourceBytes: '2809 +/- 50', sourceWastedBytes: '1259 +/- 50'}, ], diff --git a/core/audits/byte-efficiency/duplicated-javascript.js b/core/audits/byte-efficiency/duplicated-javascript.js index 399a7a1f46cf..607e2a55b7e7 100644 --- a/core/audits/byte-efficiency/duplicated-javascript.js +++ b/core/audits/byte-efficiency/duplicated-javascript.js @@ -148,7 +148,7 @@ class DuplicatedJavascript extends ByteEfficiencyAudit { const scriptId = sourceData.scriptId; const script = artifacts.Scripts.find(script => script.scriptId === scriptId); const url = script?.url || ''; - const compressionRatio = await estimateCompressionRatioForContent( + const compressionRatio = estimateCompressionRatioForContent( compressionRatioByUrl, url, artifacts, networkRecords); const transferSize = Math.round(sourceData.resourceSize * compressionRatio); diff --git a/core/audits/byte-efficiency/legacy-javascript.js b/core/audits/byte-efficiency/legacy-javascript.js index 37e22ed88a03..df5e023b8cf9 100644 --- a/core/audits/byte-efficiency/legacy-javascript.js +++ b/core/audits/byte-efficiency/legacy-javascript.js @@ -417,7 +417,7 @@ class LegacyJavascript extends ByteEfficiencyAudit { const scriptToMatchResults = this.detectAcrossScripts(matcher, artifacts.Scripts, networkRecords, bundles); for (const [script, matches] of scriptToMatchResults.entries()) { - const compressionRatio = await estimateCompressionRatioForContent( + const compressionRatio = estimateCompressionRatioForContent( compressionRatioByUrl, script.url, artifacts, networkRecords); const wastedBytes = Math.round(this.estimateWastedBytes(matches) * compressionRatio); /** @type {typeof items[number]} */ diff --git a/core/audits/byte-efficiency/unused-javascript.js b/core/audits/byte-efficiency/unused-javascript.js index f97f5b17bae9..379e709d25ef 100644 --- a/core/audits/byte-efficiency/unused-javascript.js +++ b/core/audits/byte-efficiency/unused-javascript.js @@ -8,7 +8,7 @@ import {ByteEfficiencyAudit} from './byte-efficiency-audit.js'; import {UnusedJavascriptSummary} from '../../computed/unused-javascript-summary.js'; import {JSBundles} from '../../computed/js-bundles.js'; import * as i18n from '../../lib/i18n/i18n.js'; -import {estimateTransferSize, getRequestForScript} from '../../lib/script-helpers.js'; +import {estimateCompressionRatioForContent} from '../../lib/script-helpers.js'; const UIStrings = { /** Imperative title of a Lighthouse audit that tells the user to reduce JavaScript that is never evaluated during page load. This is displayed in a list of audit titles that Lighthouse generates. */ @@ -86,26 +86,26 @@ class UnusedJavaScript extends ByteEfficiencyAudit { bundleSourceUnusedThreshold = UNUSED_BYTES_IGNORE_BUNDLE_SOURCE_THRESHOLD, } = context.options || {}; + /** @type {Map} */ + const compressionRatioByUrl = new Map(); + const items = []; for (const [scriptId, scriptCoverage] of Object.entries(artifacts.JsUsage)) { const script = artifacts.Scripts.find(s => s.scriptId === scriptId); if (!script) continue; // This should never happen. - const networkRecord = getRequestForScript(networkRecords, script); - if (!networkRecord) continue; - const bundle = bundles.find(b => b.script.scriptId === scriptId); const unusedJsSummary = await UnusedJavascriptSummary.request({scriptId, scriptCoverage, bundle}, context); if (unusedJsSummary.wastedBytes === 0 || unusedJsSummary.totalBytes === 0) continue; - const transfer = estimateTransferSize(networkRecord, unusedJsSummary.totalBytes, 'Script'); - const transferRatio = transfer / unusedJsSummary.totalBytes; + const compressionRatio = estimateCompressionRatioForContent( + compressionRatioByUrl, script.url, artifacts, networkRecords); /** @type {LH.Audit.ByteEfficiencyItem} */ const item = { url: script.url, - totalBytes: Math.round(transferRatio * unusedJsSummary.totalBytes), - wastedBytes: Math.round(transferRatio * unusedJsSummary.wastedBytes), + totalBytes: Math.round(compressionRatio * unusedJsSummary.totalBytes), + wastedBytes: Math.round(compressionRatio * unusedJsSummary.wastedBytes), wastedPercent: unusedJsSummary.wastedPercent, }; @@ -126,8 +126,8 @@ class UnusedJavaScript extends ByteEfficiencyAudit { const total = source === '(unmapped)' ? sizes.unmappedBytes : sizes.files[source]; return { source, - unused: Math.round(unused * transferRatio), - total: Math.round(total * transferRatio), + unused: Math.round(unused * compressionRatio), + total: Math.round(total * compressionRatio), }; }) .filter(d => d.unused >= bundleSourceUnusedThreshold); diff --git a/core/lib/script-helpers.js b/core/lib/script-helpers.js index ecf312e7a758..5329f5fd046e 100644 --- a/core/lib/script-helpers.js +++ b/core/lib/script-helpers.js @@ -137,7 +137,7 @@ function estimateCompressedContentSize(networkRecord, totalBytes, resourceType) * @param {LH.Artifacts} artifacts * @param {Array} networkRecords */ -async function estimateCompressionRatioForContent(compressionRatioByUrl, url, +function estimateCompressionRatioForContent(compressionRatioByUrl, url, artifacts, networkRecords) { let compressionRatio = compressionRatioByUrl.get(url); if (compressionRatio !== undefined) return compressionRatio; diff --git a/core/test/audits/byte-efficiency/unused-javascript-test.js b/core/test/audits/byte-efficiency/unused-javascript-test.js index cd04d1fc0ee9..3c4506c03737 100644 --- a/core/test/audits/byte-efficiency/unused-javascript-test.js +++ b/core/test/audits/byte-efficiency/unused-javascript-test.js @@ -30,7 +30,7 @@ function getScriptId(url) { * @param {LH.Crdp.Network.ResourceType} resourceType */ function generateRecord(url, transferSize, resourceType) { - return {url, transferSize, resourceType}; + return {url, transferSize, resourceType, responseHeaders: [{name: 'Content-Encoding'}]}; } /** @@ -38,7 +38,7 @@ function generateRecord(url, transferSize, resourceType) { * @param {Array<[number, number, number]>} ranges * @return {Crdp.Profiler.ScriptCoverage} */ -function generateUsage(url, ranges) { +function generateCoverage(url, ranges) { const functions = ranges.map(range => { return { ranges: [ @@ -54,7 +54,19 @@ function generateUsage(url, ranges) { return {scriptId: getScriptId(url), url, functions}; } -function makeJsUsage(...usages) { +/** + * @param {string} url + * @param {Array<[number, number, number]>} ranges + * @return {{script: LH.Artifacts.Script, coverage: Crdp.Profiler.ScriptCoverage}} + */ +function generateScriptWithCoverage(url, ranges) { + const length = Math.max(...ranges.map(r => r[1])); + const script = createScript({url, scriptId: getScriptId(url), length}); + const coverage = generateCoverage(url, ranges); + return {script, coverage}; +} + +function makeJsUsage(usages) { return usages.reduce((acc, cur) => { acc[cur.scriptId] = cur; return acc; @@ -63,11 +75,14 @@ function makeJsUsage(...usages) { describe('UnusedJavaScript audit', () => { const domain = 'https://www.google.com'; - const scriptUnknown = generateUsage(domain, [[0, 3000, false]]); - const scriptA = generateUsage(`${domain}/scriptA.js`, [[0, 100, true]]); - const scriptB = generateUsage(`${domain}/scriptB.js`, [[0, 200, true], [0, 50, false]]); - const inlineA = generateUsage(`${domain}/inline.html`, [[0, 5000, true], [5000, 6000, false]]); - const inlineB = generateUsage(`${domain}/inline.html`, [[0, 15000, true], [0, 5000, false]]); + const scriptUnknown = {coverage: generateCoverage(domain, [[0, 3000, false]])}; + /* eslint-disable max-len */ + const scriptA = generateScriptWithCoverage(`${domain}/scriptA.js`, [[0, 100, true]]); + const scriptB = generateScriptWithCoverage(`${domain}/scriptB.js`, [[0, 200, true], [0, 50, false]]); + const inlineA = generateScriptWithCoverage(`${domain}/inline.html`, [[0, 5000, true], [5000, 6000, false]]); + const inlineB = generateScriptWithCoverage(`${domain}/inline.html`, [[0, 15000, true], [0, 5000, false]]); + /* eslint-enable max-len */ + const all = [scriptA, scriptB, scriptUnknown, inlineA, inlineB]; const recordA = generateRecord(`${domain}/scriptA.js`, 35000, 'Script'); const recordB = generateRecord(`${domain}/scriptB.js`, 50000, 'Script'); const recordInline = generateRecord(`${domain}/inline.html`, 1000000, 'Document'); @@ -82,9 +97,8 @@ describe('UnusedJavaScript audit', () => { }; const networkRecords = [recordA, recordB, recordInline]; const artifacts = { - Scripts: [scriptA, scriptB, scriptUnknown, inlineA, inlineB] - .map((usage) => createScript({...usage, functions: undefined})), - JsUsage: makeJsUsage(scriptA, scriptB, scriptUnknown, inlineA, inlineB), + Scripts: all.map((s) => s.script).filter(Boolean), + JsUsage: makeJsUsage(all.map((s) => s.coverage)), devtoolsLogs: {defaultPass: networkRecordsToDevtoolsLog(networkRecords)}, SourceMaps: [], }; @@ -125,7 +139,7 @@ describe('UnusedJavaScript audit', () => { const url = 'https://squoosh.app/main-app.js'; const networkRecords = [generateRecord(url, content.length, 'Script')]; const artifacts = { - JsUsage: makeJsUsage(usage), + JsUsage: makeJsUsage([usage]), devtoolsLogs: {defaultPass: networkRecordsToDevtoolsLog(networkRecords)}, SourceMaps: [{scriptId: 'squoosh', scriptUrl: url, map}], Scripts: [{scriptId: 'squoosh', url, content}].map(createScript), @@ -139,7 +153,7 @@ describe('UnusedJavaScript audit', () => { "items": Array [ Object { "source": "(unmapped)", - "sourceBytes": 10062, + "sourceBytes": 10061, "sourceWastedBytes": 3760, }, Object { @@ -165,7 +179,7 @@ describe('UnusedJavaScript audit', () => { ], "type": "subitems", }, - "totalBytes": 83748, + "totalBytes": 83742, "url": "https://squoosh.app/main-app.js", "wastedBytes": 6961, "wastedPercent": 8.312435814764395, diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json index 6094b082f427..4fe48d92290c 100644 --- a/core/test/results/sample_v2.json +++ b/core/test/results/sample_v2.json @@ -4541,7 +4541,7 @@ "scoreDisplayMode": "metricSavings", "numericValue": 750, "numericUnit": "millisecond", - "displayValue": "Potential savings of 64 KiB", + "displayValue": "Potential savings of 63 KiB", "metricSavings": { "FCP": 0, "LCP": 450 @@ -4578,19 +4578,19 @@ "items": [ { "url": "http://localhost:10200/dobetterweb/third_party/aggressive-promise-polyfill.js", - "totalBytes": 166550, - "wastedBytes": 43567, + "totalBytes": 166320, + "wastedBytes": 43507, "wastedPercent": 26.158609908609908 }, { "url": "http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js", - "totalBytes": 30346, - "wastedBytes": 21544, + "totalBytes": 29671, + "wastedBytes": 21065, "wastedPercent": 70.99531129443884 } ], "overallSavingsMs": 750, - "overallSavingsBytes": 65111, + "overallSavingsBytes": 64572, "sortedBy": [ "wastedBytes" ], @@ -10449,7 +10449,7 @@ }, { "values": { - "wastedBytes": 65111 + "wastedBytes": 64572 }, "path": "audits[unused-javascript].displayValue" },