From 523576dcdd65733dae965d6bc3c66b96939acc92 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 23 Aug 2024 13:55:31 -0700 Subject: [PATCH 1/3] core(uses-text-compression): remove 10% savings threshold --- .../byte-efficiency/uses-text-compression.js | 20 ++++++---- .../uses-text-compression-test.js | 38 ++++++++----------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/core/audits/byte-efficiency/uses-text-compression.js b/core/audits/byte-efficiency/uses-text-compression.js index 6821453d6e54..e31972d1a2a0 100644 --- a/core/audits/byte-efficiency/uses-text-compression.js +++ b/core/audits/byte-efficiency/uses-text-compression.js @@ -25,7 +25,7 @@ const UIStrings = { const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); const IGNORE_THRESHOLD_IN_BYTES = 1400; -const IGNORE_THRESHOLD_IN_PERCENT = 0.1; +const IGNORE_THRESHOLD_IN_PERCENT = 0.05; class ResponsesAreCompressed extends ByteEfficiencyAudit { /** @@ -59,12 +59,18 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit { const gzipSize = record.gzipSize; const gzipSavings = originalSize - gzipSize; - // we require at least 10% savings off the original size AND at least 1400 bytes - // if the savings is smaller than either, we don't care - if (1 - gzipSize / originalSize < IGNORE_THRESHOLD_IN_PERCENT || - gzipSavings < IGNORE_THRESHOLD_IN_BYTES || - record.transferSize < gzipSize - ) { + // Not every resource is smaller when compressed. + if (record.transferSize < gzipSize) { + return; + } + + // If savings is small, let's be generous and not surface the minor savings. + if (gzipSavings < IGNORE_THRESHOLD_IN_BYTES) { + return; + } + + // Require at least 20kb of savings ... or some percentage of total resource size. + if (gzipSavings < 20_000 && 1 - gzipSize / originalSize < IGNORE_THRESHOLD_IN_PERCENT) { return; } diff --git a/core/test/audits/byte-efficiency/uses-text-compression-test.js b/core/test/audits/byte-efficiency/uses-text-compression-test.js index 3fffa1b32336..33c1664963fa 100644 --- a/core/test/audits/byte-efficiency/uses-text-compression-test.js +++ b/core/test/audits/byte-efficiency/uses-text-compression-test.js @@ -4,44 +4,36 @@ * SPDX-License-Identifier: Apache-2.0 */ -const KB = 1024; -import assert from 'assert/strict'; - import ResponsesAreCompressedAudit from '../../../audits/byte-efficiency/uses-text-compression.js'; +const KB = 1024; +const MB = 1024 * KB; + function generateResponse(options) { - return Object.assign({ + return { url: `http://google.com/${options.file}`, transferSize: options.resourceSize || 0, resourceSize: 0, gzipSize: 0, - }, options); + ...options, + }; } describe('Page uses optimized responses', () => { - it('fails when responses are collectively unoptimized', () => { - const auditResult = ResponsesAreCompressedAudit.audit_({ - ResponseCompression: [ - generateResponse({file: 'index.js', resourceSize: 100 * KB, gzipSize: 90 * KB}), // 10kb & 10% - generateResponse({file: 'index.css', resourceSize: 50 * KB, gzipSize: 37 * KB}), // 13kb & 26% (hit) - generateResponse({file: 'index.json', resourceSize: 2048 * KB, gzipSize: 1024 * KB}), // 1024kb & 50% (hit) - ], - }); - - assert.equal(auditResult.items.length, 2); - }); - - it('passes when all responses are sufficiently optimized', () => { + it('applies a threshold', () => { const auditResult = ResponsesAreCompressedAudit.audit_({ ResponseCompression: [ - generateResponse({file: 'index.js', resourceSize: 1000 * KB, gzipSize: 910 * KB}), // 90kb & 9% - generateResponse({file: 'index.css', resourceSize: 6 * KB, gzipSize: 4.5 * KB}), // 1,5kb & 25% (hit) - generateResponse({file: 'index.json', resourceSize: 10 * KB, gzipSize: 10 * KB}), // 0kb & 0% + generateResponse({file: 'index.js', resourceSize: 1000 * KB, gzipSize: 910 * KB}), // 90kb (hit) + generateResponse({file: 'index.css', resourceSize: 6 * KB, gzipSize: 4.8 * KB}), // 1.2kb + generateResponse({file: 'index2.css', resourceSize: 50 * KB, gzipSize: 37 * KB}), // 13kb (hit) + generateResponse({file: 'index.json', resourceSize: 10 * KB, gzipSize: 10 * KB}), // 0kb + generateResponse({file: 'uncompressed.xcustom', resourceSize: 11 * MB, gzipSize: 10 * MB}), // 1mb generateResponse({file: 'compressed.json', resourceSize: 10 * KB, transferSize: 3 * KB, - gzipSize: 6 * KB}), // 0kb & 0% + gzipSize: 6 * KB}), // 0kb ], }); - assert.equal(auditResult.items.length, 1); + expect(auditResult.items.map(item => item.url)).toEqual( + ['http://google.com/index.js', 'http://google.com/index2.css', 'http://google.com/uncompressed.xcustom']); }); }); From 762e97a9cd0f47c9538b80d74f870a09f8ce7dec Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 23 Aug 2024 13:56:02 -0700 Subject: [PATCH 2/3] put that back --- core/audits/byte-efficiency/uses-text-compression.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/audits/byte-efficiency/uses-text-compression.js b/core/audits/byte-efficiency/uses-text-compression.js index e31972d1a2a0..f4a360737f2e 100644 --- a/core/audits/byte-efficiency/uses-text-compression.js +++ b/core/audits/byte-efficiency/uses-text-compression.js @@ -25,7 +25,7 @@ const UIStrings = { const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); const IGNORE_THRESHOLD_IN_BYTES = 1400; -const IGNORE_THRESHOLD_IN_PERCENT = 0.05; +const IGNORE_THRESHOLD_IN_PERCENT = 0.1; class ResponsesAreCompressed extends ByteEfficiencyAudit { /** From 1009784287f3f33c08f580a6e6c42984ffcf13bf Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 23 Aug 2024 13:57:01 -0700 Subject: [PATCH 3/3] comment --- core/test/audits/byte-efficiency/uses-text-compression-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/audits/byte-efficiency/uses-text-compression-test.js b/core/test/audits/byte-efficiency/uses-text-compression-test.js index 33c1664963fa..961ec3135c90 100644 --- a/core/test/audits/byte-efficiency/uses-text-compression-test.js +++ b/core/test/audits/byte-efficiency/uses-text-compression-test.js @@ -27,7 +27,7 @@ describe('Page uses optimized responses', () => { generateResponse({file: 'index.css', resourceSize: 6 * KB, gzipSize: 4.8 * KB}), // 1.2kb generateResponse({file: 'index2.css', resourceSize: 50 * KB, gzipSize: 37 * KB}), // 13kb (hit) generateResponse({file: 'index.json', resourceSize: 10 * KB, gzipSize: 10 * KB}), // 0kb - generateResponse({file: 'uncompressed.xcustom', resourceSize: 11 * MB, gzipSize: 10 * MB}), // 1mb + generateResponse({file: 'uncompressed.xcustom', resourceSize: 11 * MB, gzipSize: 10 * MB}), // 1mb (hit) generateResponse({file: 'compressed.json', resourceSize: 10 * KB, transferSize: 3 * KB, gzipSize: 6 * KB}), // 0kb ],