Skip to content

Commit 66ce817

Browse files
Boyeepjames-elicx
andauthored
fix(build): skip unhelpful precompressed variants (#2712)
* fix(build): skip unhelpful precompressed variants * fix(build): require wire-beneficial precompression * fix(server): ignore precompression temp artifacts * fix(server): scope temp exclusion to hashed assets * test(server): keep range fixture wire-beneficial --------- Co-authored-by: James <james@eli.cx>
1 parent bf2d7ac commit 66ce817

6 files changed

Lines changed: 231 additions & 25 deletions

File tree

packages/vinext/src/build/precompress.ts

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@ import fsp from "node:fs/promises";
1313
import os from "node:os";
1414
import path from "pathslash";
1515
import zlib from "node:zlib";
16+
import { randomUUID } from "node:crypto";
1617
import { promisify } from "node:util";
1718
import { ASSET_PREFIX_URL_DIR } from "../utils/asset-prefix.js";
19+
import {
20+
isPrecompressedVariantBeneficial,
21+
type PrecompressedEncoding,
22+
} from "../utils/precompressed-variant.js";
1823

1924
const brotliCompress = promisify(zlib.brotliCompress);
2025
const gzip = promisify(zlib.gzip);
@@ -47,7 +52,10 @@ const CONCURRENCY = Math.min(os.availableParallelism(), 8);
4752
type PrecompressResult = {
4853
filesCompressed: number;
4954
totalOriginalBytes: number;
50-
/** Sum of brotli-compressed sizes (used for compression ratio reporting). */
55+
/**
56+
* Sum of the representation sizes Brotli negotiation would serve. Falls
57+
* back to the original size when Brotli is not beneficial for a file.
58+
*/
5159
totalBrotliBytes: number;
5260
};
5361

@@ -126,7 +134,10 @@ export async function precompressAssets(
126134
// readFile already done before this check — stat()-first would save
127135
// the read for tiny files but costs an extra syscall per file;
128136
// sub-1KB hashed assets are rare enough that read-first is cheaper.
129-
if (content.length < MIN_SIZE) return;
137+
if (content.length < MIN_SIZE) {
138+
await removeCompressedVariants(fullPath);
139+
return;
140+
}
130141

131142
// Compress all variants concurrently within each file
132143
const compressions: Promise<Buffer>[] = [
@@ -146,20 +157,21 @@ export async function precompressAssets(
146157
const results = await Promise.all(compressions);
147158
const [brContent, gzContent, zstdContent] = results;
148159

149-
const writes = [
150-
fsp.writeFile(fullPath + ".br", brContent),
151-
fsp.writeFile(fullPath + ".gz", gzContent),
152-
];
153-
if (zstdContent) {
154-
writes.push(fsp.writeFile(fullPath + ".zst", zstdContent));
155-
}
156-
await Promise.all(writes);
160+
const outcomes = await Promise.all([
161+
writeBeneficialVariant(fullPath + ".br", brContent, content.length, "br"),
162+
writeBeneficialVariant(fullPath + ".gz", gzContent, content.length, "gzip"),
163+
zstdContent
164+
? writeBeneficialVariant(fullPath + ".zst", zstdContent, content.length, "zstd")
165+
: removeFileIfPresent(fullPath + ".zst").then(() => false),
166+
]);
157167

158168
// Increment counters only after all writes succeed, so partial
159169
// failures (e.g. ENOSPC mid-write) don't inflate the reported totals.
160-
result.filesCompressed++;
161-
result.totalOriginalBytes += content.length;
162-
result.totalBrotliBytes += brContent.length;
170+
if (outcomes.some(Boolean)) {
171+
result.filesCompressed++;
172+
result.totalOriginalBytes += content.length;
173+
result.totalBrotliBytes += outcomes[0] ? brContent.length : content.length;
174+
}
163175
}),
164176
);
165177
// Report progress once per chunk to avoid non-deterministic ordering
@@ -172,3 +184,37 @@ export async function precompressAssets(
172184

173185
return result;
174186
}
187+
188+
async function writeBeneficialVariant(
189+
destination: string,
190+
compressed: Buffer,
191+
originalSize: number,
192+
encoding: PrecompressedEncoding,
193+
): Promise<boolean> {
194+
if (!isPrecompressedVariantBeneficial(compressed.length, originalSize, encoding)) {
195+
await removeFileIfPresent(destination);
196+
return false;
197+
}
198+
199+
const temporary = `${destination}.${process.pid}.${randomUUID()}.tmp`;
200+
try {
201+
await fsp.writeFile(temporary, compressed);
202+
await fsp.rename(temporary, destination);
203+
} catch (error) {
204+
await removeFileIfPresent(temporary);
205+
throw error;
206+
}
207+
return true;
208+
}
209+
210+
async function removeCompressedVariants(fullPath: string): Promise<void> {
211+
await Promise.all([
212+
removeFileIfPresent(fullPath + ".br"),
213+
removeFileIfPresent(fullPath + ".gz"),
214+
removeFileIfPresent(fullPath + ".zst"),
215+
]);
216+
}
217+
218+
async function removeFileIfPresent(filePath: string): Promise<void> {
219+
await fsp.rm(filePath, { force: true });
220+
}

packages/vinext/src/server/static-file-cache.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import fsp from "node:fs/promises";
1515
import path, { toSlash } from "pathslash";
1616
import { ASSET_PREFIX_URL_DIR } from "../utils/asset-prefix.js";
17+
import { isPrecompressedVariantBeneficial } from "../utils/precompressed-variant.js";
1718

1819
/** Content-type lookup for static assets. Shared with prod-server.ts. */
1920
export const CONTENT_TYPES: Record<string, string> = {
@@ -64,6 +65,10 @@ export function contentTypeForPath(filePath: string): string {
6465
*/
6566
const BUFFER_THRESHOLD = 64 * 1024;
6667

68+
/** Temp files left by an interrupted atomic precompression write. */
69+
const PRECOMPRESSION_TEMP_FILE_RE =
70+
/\.(?:br|gz|zst)\.\d+\.[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}\.tmp$/i;
71+
6772
/** A servable file variant with pre-computed response headers. */
6873
type FileVariant = {
6974
/** Absolute file path (used for streaming large files). */
@@ -136,8 +141,6 @@ export class StaticFileCache {
136141
// Skip .vite/ internal directory
137142
if (relativePath.startsWith(".vite/") || relativePath === ".vite") continue;
138143

139-
const ext = path.extname(relativePath);
140-
const contentType = contentTypeForPath(relativePath);
141144
// Files under Vite's `assetsDir` are content-hashed. The default
142145
// layout writes to `<ASSET_PREFIX_URL_DIR>/` (Next.js's canonical
143146
// convention); when `assetPrefix` is a path prefix the layout
@@ -154,6 +157,14 @@ export class StaticFileCache {
154157
const isHashed =
155158
relativePath.startsWith(`${ASSET_PREFIX_URL_DIR}/`) ||
156159
relativePath.includes(`/${ASSET_PREFIX_URL_DIR}/`);
160+
161+
// A hard process exit can strand the temporary side of an atomic
162+
// precompression write. Never register that internal artifact as a URL,
163+
// but preserve matching user files outside the precompression target.
164+
if (isHashed && PRECOMPRESSION_TEMP_FILE_RE.test(relativePath)) continue;
165+
166+
const ext = path.extname(relativePath);
167+
const contentType = contentTypeForPath(relativePath);
157168
const cacheControl = isHashed
158169
? "public, max-age=31536000, immutable"
159170
: "public, max-age=3600";
@@ -190,17 +201,17 @@ export class StaticFileCache {
190201

191202
// Pre-compute compressed variant headers (with Content-Encoding, Vary, correct Content-Length)
192203
const brInfo = allFiles.get(relativePath + ".br");
193-
if (brInfo) {
204+
if (brInfo && isPrecompressedVariantBeneficial(brInfo.size, fileInfo.size, "br")) {
194205
entry.br = buildVariant(brInfo, baseHeaders, "br");
195206
}
196207

197208
const gzInfo = allFiles.get(relativePath + ".gz");
198-
if (gzInfo) {
209+
if (gzInfo && isPrecompressedVariantBeneficial(gzInfo.size, fileInfo.size, "gzip")) {
199210
entry.gz = buildVariant(gzInfo, baseHeaders, "gzip");
200211
}
201212

202213
const zstInfo = allFiles.get(relativePath + ".zst");
203-
if (zstInfo) {
214+
if (zstInfo && isPrecompressedVariantBeneficial(zstInfo.size, fileInfo.size, "zstd")) {
204215
entry.zst = buildVariant(zstInfo, baseHeaders, "zstd");
205216
}
206217

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
export type PrecompressedEncoding = "br" | "gzip" | "zstd";
2+
3+
const VARY_HEADER_BYTES = "Vary: Accept-Encoding\r\n".length;
4+
const RESPONSE_HEADER_OVERHEAD: Record<PrecompressedEncoding, number> = {
5+
br: "Content-Encoding: br\r\n".length + VARY_HEADER_BYTES,
6+
gzip: "Content-Encoding: gzip\r\n".length + VARY_HEADER_BYTES,
7+
zstd: "Content-Encoding: zstd\r\n".length + VARY_HEADER_BYTES,
8+
};
9+
10+
/**
11+
* Return whether an encoded representation reduces conservative HTTP/1 wire
12+
* bytes after accounting for the response fields needed to negotiate it.
13+
*/
14+
export function isPrecompressedVariantBeneficial(
15+
compressedSize: number,
16+
originalSize: number,
17+
encoding: PrecompressedEncoding,
18+
): boolean {
19+
return originalSize - compressedSize > RESPONSE_HEADER_OVERHEAD[encoding];
20+
}

tests/precompress.test.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,32 @@ import fs from "node:fs";
1111
import path from "node:path";
1212
import os from "node:os";
1313
import zlib from "node:zlib";
14+
import { createHash } from "node:crypto";
1415
import { precompressAssets } from "../packages/vinext/src/build/precompress.js";
16+
import { isPrecompressedVariantBeneficial } from "../packages/vinext/src/utils/precompressed-variant.js";
1517

1618
/** Write a file with repeated content to ensure it exceeds compression threshold. */
17-
async function writeAsset(clientDir: string, relativePath: string, content: string): Promise<void> {
19+
async function writeAsset(
20+
clientDir: string,
21+
relativePath: string,
22+
content: string | Uint8Array,
23+
): Promise<void> {
1824
const fullPath = path.join(clientDir, relativePath);
1925
await fsp.mkdir(path.dirname(fullPath), { recursive: true });
2026
await fsp.writeFile(fullPath, content);
2127
}
2228

29+
function deterministicHighEntropyBytes(size: number): Buffer {
30+
const chunks: Buffer[] = [];
31+
let bytes = 0;
32+
for (let index = 0; bytes < size; index++) {
33+
const chunk = createHash("sha256").update(`vinext-precompress-${index}`).digest();
34+
chunks.push(chunk);
35+
bytes += chunk.length;
36+
}
37+
return Buffer.concat(chunks).subarray(0, size);
38+
}
39+
2340
describe("precompressAssets", () => {
2441
let clientDir: string;
2542

@@ -82,6 +99,64 @@ describe("precompressAssets", () => {
8299
expect(result.filesCompressed).toBe(0);
83100
});
84101

102+
it("does not emit compressed representations that are not smaller", async () => {
103+
const content = deterministicHighEntropyBytes(4096);
104+
await writeAsset(clientDir, "_next/static/random-aaa111.wasm", content);
105+
106+
const result = await precompressAssets(clientDir);
107+
108+
expect(result.filesCompressed).toBe(0);
109+
expect(fs.existsSync(path.join(clientDir, "_next/static/random-aaa111.wasm.br"))).toBe(false);
110+
expect(fs.existsSync(path.join(clientDir, "_next/static/random-aaa111.wasm.gz"))).toBe(false);
111+
expect(fs.existsSync(path.join(clientDir, "_next/static/random-aaa111.wasm.zst"))).toBe(false);
112+
});
113+
114+
it("does not emit a marginally smaller representation with larger wire overhead", async () => {
115+
const content = deterministicHighEntropyBytes(4096);
116+
content.copy(content, content.length - 48, 0, 48);
117+
const brotli = zlib.brotliCompressSync(content, {
118+
params: { [zlib.constants.BROTLI_PARAM_QUALITY]: 5 },
119+
});
120+
expect(brotli.length).toBeLessThan(content.length);
121+
expect(isPrecompressedVariantBeneficial(brotli.length, content.length, "br")).toBe(false);
122+
123+
const relativePath = "_next/static/marginal-aaa111.wasm";
124+
await writeAsset(clientDir, relativePath, content);
125+
await precompressAssets(clientDir);
126+
127+
expect(fs.existsSync(path.join(clientDir, relativePath + ".br"))).toBe(false);
128+
});
129+
130+
it("removes obsolete compressed representations on a later run", async () => {
131+
const relativePath = "_next/static/changing-aaa111.wasm";
132+
const fullPath = path.join(clientDir, relativePath);
133+
await writeAsset(clientDir, relativePath, "compressible\n".repeat(500));
134+
await precompressAssets(clientDir);
135+
expect(fs.existsSync(fullPath + ".br")).toBe(true);
136+
137+
await writeAsset(clientDir, relativePath, deterministicHighEntropyBytes(4096));
138+
await precompressAssets(clientDir);
139+
140+
expect(fs.existsSync(fullPath + ".br")).toBe(false);
141+
expect(fs.existsSync(fullPath + ".gz")).toBe(false);
142+
expect(fs.existsSync(fullPath + ".zst")).toBe(false);
143+
});
144+
145+
it("removes obsolete variants when an asset falls below the size threshold", async () => {
146+
const relativePath = "_next/static/shrinking-aaa111.js";
147+
const fullPath = path.join(clientDir, relativePath);
148+
await writeAsset(clientDir, relativePath, "const value = 1;\n".repeat(500));
149+
await precompressAssets(clientDir);
150+
expect(fs.existsSync(fullPath + ".br")).toBe(true);
151+
152+
await writeAsset(clientDir, relativePath, "const value = 1;");
153+
await precompressAssets(clientDir);
154+
155+
expect(fs.existsSync(fullPath + ".br")).toBe(false);
156+
expect(fs.existsSync(fullPath + ".gz")).toBe(false);
157+
expect(fs.existsSync(fullPath + ".zst")).toBe(false);
158+
});
159+
85160
it("skips non-compressible file types (images, fonts)", async () => {
86161
// PNG file (binary, already compressed)
87162
const pngHeader = Buffer.alloc(2048, 0x89); // fake PNG data, above threshold
@@ -229,3 +304,17 @@ describe("precompressAssets", () => {
229304
expect(result.totalBrotliBytes).toBeLessThan(result.totalOriginalBytes);
230305
});
231306
});
307+
308+
describe("isPrecompressedVariantBeneficial", () => {
309+
it.each([
310+
["br", 45],
311+
["gzip", 47],
312+
["zstd", 47],
313+
] as const)(
314+
"requires %s to save more than its response-header overhead",
315+
(encoding, overhead) => {
316+
expect(isPrecompressedVariantBeneficial(1000 - overhead, 1000, encoding)).toBe(false);
317+
expect(isPrecompressedVariantBeneficial(999 - overhead, 1000, encoding)).toBe(true);
318+
},
319+
);
320+
});

tests/serve-static.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,9 @@ describe("tryServeStatic (with StaticFileCache)", () => {
832832

833833
it("serves cached ranges with conditional precedence and lossless large integers", async () => {
834834
const relativePath = "_next/static/range-aaa111.js";
835-
const content = "0123456789";
835+
// Keep the compressed sidecar wire-beneficial so this still exercises
836+
// range negotiation against an entry that varies by Accept-Encoding.
837+
const content = "0123456789".repeat(100);
836838
await writeFile(clientDir, relativePath, content);
837839
await writeFile(clientDir, `${relativePath}.br`, zlib.brotliCompressSync(content));
838840
const cache = await StaticFileCache.create(clientDir);
@@ -864,11 +866,11 @@ describe("tryServeStatic (with StaticFileCache)", () => {
864866
await tryServeStatic(rangeReq, rangeRes, clientDir, `/${relativePath}`, true, cache);
865867
await range.ended;
866868
expect(range.status).toBe(206);
867-
expect(range.headers["Content-Range"]).toBe("bytes 2-9/10");
868-
expect(range.headers["Content-Length"]).toBe("8");
869+
expect(range.headers["Content-Range"]).toBe("bytes 2-999/1000");
870+
expect(range.headers["Content-Length"]).toBe("998");
869871
expect(range.headers["Content-Encoding"]).toBeUndefined();
870872
expect(range.headers.Vary).toBe("Accept-Encoding");
871-
expect(range.body.toString()).toBe("23456789");
873+
expect(range.body.toString()).toBe(content.slice(2));
872874

873875
const unsatisfiableReq = mockReq(undefined, { range: "bytes=9007199254740992-" });
874876
const { res: unsatisfiableRes, captured: unsatisfiable } = mockRes();
@@ -883,15 +885,15 @@ describe("tryServeStatic (with StaticFileCache)", () => {
883885
await unsatisfiable.ended;
884886
expect(unsatisfiable.status).toBe(416);
885887
expect(unsatisfiable.headers["Content-Type"]).toBe("application/javascript; charset=utf-8");
886-
expect(unsatisfiable.headers["Content-Range"]).toBe("bytes */10");
888+
expect(unsatisfiable.headers["Content-Range"]).toBe("bytes */1000");
887889
expect(unsatisfiable.body).toHaveLength(0);
888890

889891
const headReq = mockReq(undefined, { range: "bytes=2-5" }, "HEAD");
890892
const { res: headRes, captured: head } = mockRes();
891893
await tryServeStatic(headReq, headRes, clientDir, `/${relativePath}`, true, cache);
892894
await head.ended;
893895
expect(head.status).toBe(206);
894-
expect(head.headers["Content-Range"]).toBe("bytes 2-5/10");
896+
expect(head.headers["Content-Range"]).toBe("bytes 2-5/1000");
895897
expect(head.headers["Content-Length"]).toBe("4");
896898
expect(head.body).toHaveLength(0);
897899
});

tests/static-file-cache.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,44 @@ describe("StaticFileCache", () => {
9090
expect(cache.lookup("/_next/static/missing-xyz789.js")).toBeUndefined();
9191
});
9292

93+
it("ignores precompressed variants that are not smaller than the original", async () => {
94+
await writeFile(clientDir, "_next/static/app-abc123.js", "small original");
95+
await writeFile(clientDir, "_next/static/app-abc123.js.br", "larger brotli representation");
96+
97+
const cache = await StaticFileCache.create(clientDir);
98+
const entry = cache.lookup("/_next/static/app-abc123.js");
99+
100+
expect(entry?.br).toBeUndefined();
101+
expect(entry?.original.headers.Vary).toBeUndefined();
102+
});
103+
104+
it("ignores variants whose savings do not cover response-header overhead", async () => {
105+
await writeFile(clientDir, "_next/static/app-abc123.js", "x".repeat(1000));
106+
await writeFile(clientDir, "_next/static/app-abc123.js.br", "y".repeat(955));
107+
await writeFile(clientDir, "_next/static/app-abc123.js.gz", "z".repeat(953));
108+
109+
const cache = await StaticFileCache.create(clientDir);
110+
const entry = cache.lookup("/_next/static/app-abc123.js");
111+
112+
expect(entry?.br).toBeUndefined();
113+
expect(entry?.gz).toBeUndefined();
114+
expect(entry?.original.headers.Vary).toBeUndefined();
115+
});
116+
117+
it("does not serve temporary files left by interrupted precompression", async () => {
118+
const basename = "app-abc123.js.br.123.123e4567-e89b-42d3-a456-426614174000.tmp";
119+
const temporaryPath = `_next/static/${basename}`;
120+
await writeFile(clientDir, temporaryPath, "partial compressed content");
121+
await writeFile(clientDir, `downloads/${basename}`, "legitimate public content");
122+
123+
const cache = await StaticFileCache.create(clientDir);
124+
125+
expect(cache.lookup("/" + temporaryPath)).toBeUndefined();
126+
expect(cache.lookup(`/downloads/${basename}`)?.original.buffer?.toString()).toBe(
127+
"legitimate public content",
128+
);
129+
});
130+
93131
it("sets immutable cache-control for hashed assets under /assets/", async () => {
94132
await writeFile(clientDir, "_next/static/bundle-abc123.js", "x".repeat(100));
95133

0 commit comments

Comments
 (0)