perf(sourcemap-upload): skip ZIP-level DEFLATE when wire codec compresses (62% CPU, 5.9% wire)

src/lib/api/sourcemaps.ts

@@ -31,7 +31,7 @@ import { ApiError } from "../errors.js";
 import { logger } from "../logger.js";
 import { resolveOrgRegion } from "../region.js";
 import { getSdkConfig } from "../sentry-client.js";
-import { ZipWriter } from "../sourcemap/zip.js";
+import { type ZipCompression, ZipWriter } from "../sourcemap/zip.js";
 import { apiRequestToRegion } from "./infrastructure.js";
 
 const gzipAsync = promisify(gzipCb);
@@ -288,12 +288,22 @@ async function uploadChunk(params: {
  *
  * @param outputPath - Where to write the ZIP file
  * @param files - Source files and sourcemaps to include
- * @param options - Bundle metadata (org, project, release)
+ * @param options.org / project / release - Bundle metadata
+ * @param options.compression - ZIP entry compression. Use `"stored"`
+ *   when the wire layer will compress the chunks (zstd or gzip);
+ *   compressing twice wastes CPU and barely helps wire size.
+ *   Defaults to `"deflate"` so callers without a wire codec still
+ *   ship reasonably-sized payloads.
  */
 export async function buildArtifactBundle(
   outputPath: string,
   files: ArtifactFile[],
-  options: { org: string; project: string; release?: string }
+  options: {
+    org: string;
+    project: string;
+    release?: string;
+    compression?: ZipCompression;
+  }
 ): Promise<void> {
   // Build manifest.json
   const filesManifest: Record<
@@ -329,7 +339,9 @@ export async function buildArtifactBundle(
   });
 
   // Stream ZIP entries to disk
-  const zip = await ZipWriter.create(outputPath);
+  const zip = await ZipWriter.create(outputPath, {
+    compression: options.compression,
+  });
   try {
     await zip.addEntry("manifest.json", Buffer.from(manifest, "utf-8"));
 
@@ -396,16 +408,31 @@ export async function uploadSourcemaps(options: UploadOptions): Promise<void> {
   // Step 1: Get chunk upload configuration
   const serverOptions = await getChunkUploadOptions(org);
 
+  // Pick the wire codec up-front so the ZIP can skip its own
+  // compression pass when the wire layer will handle it. Re-compressing
+  // an already-DEFLATE'd ZIP with zstd/gzip burns CPU for ~0% wire
+  // savings; STORED + zstd saves both CPU and a few percent wire bytes.
+  // Without a wire codec (kill-switch / unsupported codecs) we keep
+  // DEFLATE so the ZIP itself stays small.
+  const encoding = pickUploadEncoding(serverOptions.compression);
+  const zipCompression: ZipCompression = encoding ? "stored" : "deflate";
+
   // Step 2: Build artifact bundle ZIP to a temp file, then upload
   const tmpZipPath = join(tmpdir(), `sentry-artifact-bundle-${Date.now()}.zip`);
   try {
-    await buildArtifactBundle(tmpZipPath, files, { org, project, release });
+    await buildArtifactBundle(tmpZipPath, files, {
+      org,
+      project,
+      release,
+      compression: zipCompression,
+    });
     await uploadArtifactBundle({
       tmpZipPath,
       org,
       project,
       release,
       serverOptions,
+      encoding,
     });
   } finally {
     // Always clean up the temp file
@@ -428,8 +455,9 @@ async function uploadArtifactBundle(opts: {
   project: string;
   release: string | undefined;
   serverOptions: ChunkServerOptions;
+  encoding: UploadEncoding | undefined;
 }): Promise<void> {
-  const { tmpZipPath, org, project, release, serverOptions } = opts;
+  const { tmpZipPath, org, project, release, serverOptions, encoding } = opts;
   // Step 3: Split into chunks, hash each chunk + compute overall checksum
   const { chunks, overallChecksum } = await hashChunks(
     tmpZipPath,
@@ -477,7 +505,6 @@ async function uploadArtifactBundle(opts: {
   const missingChunks = chunks.filter((c) => missingSet.has(c.sha1));
 
   if (missingChunks.length > 0) {
-    const encoding = pickUploadEncoding(serverOptions.compression);
     const limit = pLimit(serverOptions.concurrency);
     // Use the CLI's authenticated fetch for chunk uploads
     const { fetch: authFetch } = getSdkConfig(regionUrl);

src/lib/sourcemap/zip.ts

@@ -5,8 +5,10 @@
  * compressed data is held in memory at a time. Produces valid ZIP
  * archives that can be extracted by standard tools (unzip, 7z, etc.).
  *
- * Uses raw DEFLATE (method 8) for compression and the CRC-32
- * function from `node:zlib` for checksums.
+ * Per-archive compression method (DEFLATE or STORED) is chosen at
+ * {@link ZipWriter.create} time. STORED is preferred when the
+ * caller will compress the bytes again at the wire layer (e.g.
+ * chunk-upload with zstd) — see {@link ZipCompression}.
  */
 
 import type { FileHandle } from "node:fs/promises";
@@ -57,13 +59,23 @@ type EntryRecord = {
   localHeaderOffset: number;
 };
 
+/**
+ * Per-archive compression strategy. `"deflate"` mirrors the original
+ * behavior; `"stored"` skips entry compression entirely and is the
+ * right choice when the bytes will be re-compressed by an outer codec
+ * (e.g. chunk-upload with zstd/gzip on the wire). Compressing an
+ * already-compressed payload is wasted CPU and can even slightly
+ * inflate the output.
+ */
+export type ZipCompression = "deflate" | "stored";
+
 /**
  * Streaming ZIP archive writer.
  *
- * Entries are compressed and flushed to disk one at a time via
- * {@link addEntry}, keeping peak memory proportional to a single
- * file's compressed output. Call {@link finalize} after all entries
- * have been added to write the central directory and close the file.
+ * Entries are written to disk one at a time via {@link addEntry},
+ * keeping peak memory proportional to a single entry's payload. Call
+ * {@link finalize} after all entries have been added to write the
+ * central directory and close the file.
  *
  * @example
  * ```ts
@@ -82,8 +94,11 @@ export class ZipWriter {
 
   private readonly fh: FileHandle;
 
-  private constructor(fh: FileHandle) {
+  private readonly compression: ZipCompression;
+
+  private constructor(fh: FileHandle, compression: ZipCompression) {
     this.fh = fh;
+    this.compression = compression;
   }
 
   /**
@@ -100,11 +115,17 @@ export class ZipWriter {
    * valid archive and release the file handle.
    *
    * @param outputPath - Filesystem path for the output ZIP file.
+   * @param options.compression - Per-entry compression method
+   *   (`"deflate"` default; `"stored"` to skip compression — see
+   *   {@link ZipCompression}).
    * @returns A ready-to-use writer instance.
    */
-  static async create(outputPath: string): Promise<ZipWriter> {
+  static async create(
+    outputPath: string,
+    options: { compression?: ZipCompression } = {}
+  ): Promise<ZipWriter> {
     const fh = await open(outputPath, "w");
-    const writer = new ZipWriter(fh);
+    const writer = new ZipWriter(fh, options.compression ?? "deflate");
 
     // Write the SourceBundle magic header before any ZIP data.
     // Format: 4-byte magic "SYSB" + 4-byte LE version (2).
@@ -136,9 +157,9 @@ export class ZipWriter {
   /**
    * Add a file entry to the archive.
    *
-   * The data is compressed with raw DEFLATE and written to disk
-   * immediately, so only one entry's compressed payload is buffered
-   * at a time.
+   * The data is written using the writer's configured compression
+   * method (DEFLATE or STORED). Empty entries are always STORED
+   * because some extractors mishandle DEFLATE of empty input.
    *
    * @param name - File path inside the archive (forward-slash separated).
    * @param data - Uncompressed file contents.
@@ -147,8 +168,7 @@ export class ZipWriter {
     const nameBytes = Buffer.from(name, "utf-8");
     const checksum = crc32(data);
 
-    // Use STORE for empty files (DEFLATE of empty input can confuse extractors)
-    const useStore = data.length === 0;
+    const useStore = this.compression === "stored" || data.length === 0;
     const method = useStore ? METHOD_STORE : METHOD_DEFLATE;
     const payload = useStore ? data : await deflateRaw(data);
 

test/lib/api/sourcemaps.test.ts

@@ -1,6 +1,10 @@
-import { describe, expect, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { gunzipSync } from "node:zlib";
 import {
+  buildArtifactBundle,
   ChunkServerOptionsSchema,
   encodeChunk,
   pickUploadEncoding,
@@ -57,6 +61,97 @@ describe("encodeChunk", () => {
   });
 });
 
+describe("buildArtifactBundle", () => {
+  // ZIP local file header format: at offset 8 (LE u16) is the
+  // compression method (0 = STORED, 8 = DEFLATE). The CLI prefixes
+  // every artifact bundle with an 8-byte SYSB SourceBundle header.
+  const SYSB_HEADER_BYTES = 8;
+  const LOCAL_HEADER_METHOD_OFFSET = SYSB_HEADER_BYTES + 8;
+
+  let tmpDir: string;
+
+  beforeEach(async () => {
+    tmpDir = await mkdtemp(join(tmpdir(), "bundle-test-"));
+  });
+
+  afterEach(async () => {
+    await rm(tmpDir, { recursive: true, force: true });
+  });
+
+  async function makePair(): Promise<
+    {
+      path: string;
+      debugId: string;
+      type: "minified_source" | "source_map";
+      url: string;
+      sourcemapFilename?: string;
+    }[]
+  > {
+    const jsPath = join(tmpDir, "app.js");
+    const mapPath = join(tmpDir, "app.js.map");
+    // Highly redundant content so DEFLATE has work to do — the size
+    // assertions below depend on this.
+    await writeFile(jsPath, "console.log('hello');\n".repeat(500));
+    await writeFile(mapPath, JSON.stringify({ version: 3, sources: [] }));
+    return [
+      {
+        path: jsPath,
+        debugId: "00000000-0000-0000-0000-000000000001",
+        type: "minified_source" as const,
+        url: "~/app.js",
+        sourcemapFilename: "app.js.map",
+      },
+      {
+        path: mapPath,
+        debugId: "00000000-0000-0000-0000-000000000001",
+        type: "source_map" as const,
+        url: "~/app.js.map",
+      },
+    ];
+  }
+
+  test("default compression is DEFLATE", async () => {
+    const files = await makePair();
+    const out = join(tmpDir, "bundle-default.zip");
+    await buildArtifactBundle(out, files, { org: "o", project: "p" });
+
+    const bytes = await readFile(out);
+    expect(bytes.readUInt16LE(LOCAL_HEADER_METHOD_OFFSET)).toBe(8);
+  });
+
+  test("compression: 'stored' writes entries uncompressed", async () => {
+    const files = await makePair();
+    const out = join(tmpDir, "bundle-stored.zip");
+    await buildArtifactBundle(out, files, {
+      org: "o",
+      project: "p",
+      compression: "stored",
+    });
+
+    const bytes = await readFile(out);
+    expect(bytes.readUInt16LE(LOCAL_HEADER_METHOD_OFFSET)).toBe(0);
+  });
+
+  test("STORED archive is larger than DEFLATE for the same redundant input", async () => {
+    const files = await makePair();
+    const deflateOut = join(tmpDir, "bundle-deflate.zip");
+    const storedOut = join(tmpDir, "bundle-stored-size.zip");
+    await buildArtifactBundle(deflateOut, files, { org: "o", project: "p" });
+    await buildArtifactBundle(storedOut, files, {
+      org: "o",
+      project: "p",
+      compression: "stored",
+    });
+
+    const deflateSize = (await readFile(deflateOut)).length;
+    const storedSize = (await readFile(storedOut)).length;
+    // Sanity: redundant input should DEFLATE meaningfully smaller than
+    // STORED. If this ever fails, either the ZIP layout changed or the
+    // payload stopped being compressible.
+    expect(storedSize).toBeGreaterThan(deflateSize * 2);
+  });
+});
+
 describe("ChunkServerOptionsSchema", () => {
   test("accepts compression: [] (server opt-out)", () => {
     const result = ChunkServerOptionsSchema.safeParse({