Cut the dev gate's memory and disk cost, and a third of its wall clock

src/registry/npm.rs

@@ -298,18 +298,42 @@ fn packument_size_estimate(p: &Packument) -> u64 {
             // shape. Omitting it made a packument carrying a 32 MiB block
             // measure 1050 bytes, and a 64 MiB ceiling that does not bind is
             // worse than no ceiling: it looks like a bound.
-            let signatures = v
-                .dist
-                .signatures
-                .as_ref()
-                .and_then(|sig| serde_json::to_string(sig).ok())
-                .map_or(0, |s| s.len() as u64);
+            let signatures = v.dist.signatures.as_ref().map_or(0, serialised_len);
             512 + v.dist.tarball.len() as u64 + signatures
         })
         .sum();
     p.name.len() as u64 + 512 + versions
 }
 
+/// Serialised length of a value without building the string.
+///
+/// `serde_json::to_string(..).len()` is the obvious spelling and it allocates
+/// the whole document to measure it: a packument carrying a 64 MiB signature
+/// block paid 64 MiB of transient `String` on every call, on the hot path of
+/// every memo insert. `to_writer` runs the same `CompactFormatter` over the
+/// same `Value`, so the byte count is identical -- `String`, `Vec<u8>` and a
+/// counting sink are the same stream -- and only the destination differs.
+///
+/// Serialisation can still fail on a `Value` that cannot be written, so a
+/// failed write counts as zero exactly as the old `.ok()` did.
+fn serialised_len(v: &serde_json::Value) -> u64 {
+    struct CountingWriter(u64);
+
+    impl std::io::Write for CountingWriter {
+        fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+            self.0 += buf.len() as u64;
+            Ok(buf.len())
+        }
+
+        fn flush(&mut self) -> std::io::Result<()> {
+            Ok(())
+        }
+    }
+
+    let mut counter = CountingWriter(0);
+    serde_json::to_writer(&mut counter, v).map_or(0, |()| counter.0)
+}
+
 #[derive(Default)]
 struct PackumentMemo {
     by_name: std::collections::HashMap<String, Packument>,
@@ -471,6 +495,18 @@ mod tests {
     /// sha512 SRI of the bytes "tarball-content".
     const TEST_SRI: &str = "sha512-dWJ6JIJkmHG8N3fH1b/hbpmBQ7wKIpEw3zsVl2873OtFXh9QhR1KUU3uojohuIJ/xd+hb1R0q/57C8sMt4tstQ==";
 
+    /// One signature block in the over-ceiling packument fixture, and how much
+    /// the fixture carries in total: 64 versions x 2 blocks x 768 KiB = 96 MiB,
+    /// against a 64 MiB per-document ceiling.
+    ///
+    /// Enough margin that the refusal the fixture asserts cannot be an
+    /// off-by-a-bit, and small enough that building and serialising it costs a
+    /// fraction of the 256 MiB this replaced. What that size was carrying, and
+    /// what still pins the ceiling without it, is in
+    /// `the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf`.
+    const SIGNATURE_BLOCK_BYTES: usize = 768 * 1024;
+    const SIGNATURE_FIXTURE_BYTES: u64 = 64 * 2 * SIGNATURE_BLOCK_BYTES as u64;
+
     #[test]
     fn list_versions_orders_and_limits() {
         let versions: BTreeMap<String, VersionMeta> =
@@ -1232,9 +1268,30 @@ mod tests {
     /// and the memo as a whole. A packument over the first is not cached at
     /// all; exceeding the second clears what is held, so a recursive review
     /// cannot grow the memo without limit.
+    ///
+    /// The fixture carries [`SIGNATURE_FIXTURE_BYTES`] of signature payload, which
+    /// is 1.5x the per-document ceiling. It used to be 256 MiB (4x), and that
+    /// size was not carrying more coverage:
+    ///
+    /// * the ceiling constants are pinned as values by
+    ///   `the_packument_memo_ceilings_are_the_documented_sizes`, so a mutant
+    ///   that changes `64 * 1024 * 1024` dies there whatever this fixture is;
+    /// * the one-byte comparison (`>` against `>=`) is pinned by
+    ///   `the_per_document_ceiling_is_strict`, which passes the ceilings in and
+    ///   needs a 1057-byte document -- its own comment says the real ceiling
+    ///   "cannot express this without a 64 MiB fixture";
+    /// * the whole-memo ceiling, including sum-versus-product, is pinned by
+    ///   `the_whole_memo_ceiling_is_strict_and_adds_rather_than_multiplies`;
+    /// * the estimate's own arithmetic is pinned by `the_packument_estimate_is_exact`.
+    ///
+    /// What this fixture is for is the wiring: a large, many-version packument
+    /// that goes through the production `insert` with the real ceiling and is
+    /// refused. It still does that. What it no longer buys is a 4x margin over
+    /// the ceiling, which mattered only against a mutant that raised the
+    /// constant *and* the assertion pinning it in the same build.
     #[test]
     fn the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf() {
-        let big = "x".repeat(2 * 1024 * 1024);
+        let block = "x".repeat(SIGNATURE_BLOCK_BYTES);
         let mut versions = BTreeMap::new();
         for i in 0..64 {
             versions.insert(
@@ -1245,7 +1302,7 @@ mod tests {
                     dist: Dist {
                         tarball: "https://example.invalid/p.tgz".into(),
                         integrity: None,
-                        signatures: Some(serde_json::json!([big, big])),
+                        signatures: Some(serde_json::json!([block, block])),
                     },
                 },
             );
@@ -1257,8 +1314,9 @@ mod tests {
         };
         let size = packument_size_estimate(&p);
         assert!(
-            size > 200 * 1024 * 1024,
-            "a packument carrying 256 MiB of signature blocks must not estimate to {size} bytes"
+            size >= SIGNATURE_FIXTURE_BYTES,
+            "a packument carrying {} signature bytes must not estimate to {size} bytes",
+            SIGNATURE_FIXTURE_BYTES
         );
 
         // Over the per-document ceiling: not cached.

tests/support/tarball.rs

@@ -133,7 +133,11 @@ impl Entry {
 
 /// Gzip a raw tar stream: the shape every npm tarball arrives in.
 pub fn archive(entries: &[Entry]) -> Vec<u8> {
-    let mut tar: Vec<u8> = Vec::new();
+    // Written straight into the encoder. The entry-count fixture carries
+    // 100_001 headers and the bomb carries 200 MiB of payload, so staging the
+    // tar in a `Vec` first held a second full copy of every fixture (the bomb
+    // peaked at 209 MB) and copied it twice before the encoder ever saw it.
+    let mut encoder = flate2::write::GzEncoder::new(Vec::new(), fixture_compression());
     for entry in entries {
         let size = match &entry.payload {
             Payload::Bytes(b) => b.len() as u64,
@@ -143,43 +147,59 @@ pub fn archive(entries: &[Entry]) -> Vec<u8> {
             EntryKind::Symlink | EntryKind::Hardlink => entry.link_target.as_str(),
             _ => "",
         };
-        tar.extend_from_slice(&ustar_header(
-            &entry.path,
-            entry.kind,
-            size,
-            link,
-            entry.mode,
-        ));
+        encoder
+            .write_all(&ustar_header(
+                &entry.path,
+                entry.kind,
+                size,
+                link,
+                entry.mode,
+            ))
+            .expect("write ustar header into fixture tarball");
         match &entry.payload {
             Payload::Bytes(bytes) => {
-                tar.extend_from_slice(bytes);
-                pad_to_block(&mut tar, bytes.len() as u64);
+                encoder
+                    .write_all(bytes)
+                    .expect("write payload into fixture tarball");
+                pad_to_block(&mut encoder, bytes.len() as u64);
             }
             Payload::Zeroes { len, chunk } => {
                 let mut written: u64 = 0;
                 while written < *len {
                     let take = chunk.len().min((*len - written) as usize);
-                    tar.extend_from_slice(&chunk[..take]);
+                    encoder
+                        .write_all(&chunk[..take])
+                        .expect("write zeroes into fixture tarball");
                     written += take as u64;
                 }
-                pad_to_block(&mut tar, *len);
+                pad_to_block(&mut encoder, *len);
             }
         }
     }
-    tar.extend_from_slice(&[0u8; 1024]);
-
-    let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
-    encoder.write_all(&tar).expect("gzip fixture tarball");
+    encoder.write_all(&[0u8; 1024]).expect("write tar trailer");
     encoder.finish().expect("finish gzip fixture tarball")
 }
 
+/// Compression level for fixture tarballs.
+///
+/// `fast` rather than `default`, and nothing here can tell the difference:
+/// every consumer of a fixture infers it, and the only ratio any test asserts
+/// is that a bomb of zeroes still arrives under 1 MiB, which holds at every
+/// level. Level 6 on 100 MiB of synthetic tar in an `opt-level = 0` build was
+/// the whole cost of `tar_entry_count_over_the_cap_is_refused`: 16.01 s of
+/// which the review under test was a fraction.
+fn fixture_compression() -> flate2::Compression {
+    flate2::Compression::fast()
+}
+
 /// Zero padding to the next 512-byte boundary.
 pub fn pad_to_512(len: usize) -> Vec<u8> {
     vec![0u8; (512 - len % 512) % 512]
 }
 
-fn pad_to_block(buf: &mut Vec<u8>, written: u64) {
-    buf.extend_from_slice(&pad_to_512(written as usize));
+fn pad_to_block<W: std::io::Write>(buf: &mut W, written: u64) {
+    buf.write_all(&pad_to_512(written as usize))
+        .expect("write tar block padding");
 }
 
 /// A raw ustar header with an explicit typeflag, for the archive shapes
@@ -238,7 +258,7 @@ fn ustar_header_raw(name: &str, typeflag: u8, size: u64, link_target: &str, mode
 /// Gzip raw tar bytes: the wire shape of every npm tarball. Paired with
 /// [`raw_header`] for archive shapes a builder cannot express.
 pub fn gzip(raw: &[u8]) -> Vec<u8> {
-    let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
+    let mut encoder = flate2::write::GzEncoder::new(Vec::new(), fixture_compression());
     encoder.write_all(raw).expect("gzip fixture tarball");
     encoder.finish().expect("finish gzip fixture tarball")
 }