perf: parse records in linear time instead of quadratic (#649)

Author: harlan-zw

packages/sitemapd/src/parse/index.ts

@@ -275,6 +275,18 @@ function extractFeedMetadata(
   return undefined
 }
 
+/**
+ * Case-insensitive `startsWith` that only lowercases the prefix-length slice.
+ *
+ * `source.toLowerCase().startsWith(prefix)` copies the WHOLE remaining document
+ * to compare a handful of characters, and the record loop calls it once per
+ * entry, so it is O(entries x document length) on its own.
+ */
+function startsWithCI(input: string, prefix: string): boolean {
+  return input.length >= prefix.length
+    && input.slice(0, prefix.length).toLowerCase() === prefix.toLowerCase()
+}
+
 function extractRecord(
   input: string,
   root: RootState,
@@ -284,12 +296,12 @@ function extractRecord(
   if (source.startsWith('<!--') || source.startsWith('<?'))
     return undefined
   const rootClose = `</${root.qualifiedName}>`
-  if (source.toLowerCase().startsWith(rootClose.toLowerCase()))
+  if (startsWithCI(source, rootClose))
     return { _tag: 'close', rest: source.slice(rootClose.length) }
-  if (root.name === 'rss' && source.toLowerCase().startsWith('</channel>')) {
+  if (root.name === 'rss' && startsWithCI(source, '</channel>')) {
     const rest = trimMarkupPrefix(source.slice('</channel>'.length))
     const rssClose = `</${root.qualifiedName}>`
-    if (rest.toLowerCase().startsWith(rssClose.toLowerCase()))
+    if (startsWithCI(rest, rssClose))
       return { _tag: 'close', rest: rest.slice(rssClose.length) }
     if (rest.length > 0 && findMarkupEnd(rest, 0) !== -1)
       return { _tag: 'malformed', detail: 'RSS channel is not followed by its closing root' }
@@ -335,23 +347,48 @@ function extractRecord(
   }
 }
 
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+}
+
+/**
+ * Locate a record's closing tag, skipping any comment or CDATA section that
+ * contains a lookalike.
+ *
+ * Two costs here used to scale with the whole remaining document on EVERY
+ * record, which made parsing quadratic in document size (a 40,000-entry, 8.8MB
+ * sitemap took ~229s):
+ *
+ *  1. `input.toLowerCase()` copied the entire remaining buffer per call, purely
+ *     to do a case-insensitive search. A sticky case-insensitive RegExp scans in
+ *     place instead.
+ *  2. `indexOf('<!--', cursor)` and `indexOf('<![CDATA[', cursor)` were
+ *     UNBOUNDED. Ordinary sitemaps contain neither, so both ran to the end of
+ *     the buffer just to report "absent" — once per record. They are now bounded
+ *     to `[cursor, closeIndex)`, the only window where a hidden section could
+ *     actually affect which close tag is real.
+ *
+ * Behaviour is unchanged; only the complexity is. Same file, 8.8MB: ~0.55s.
+ */
 function findRecordClose(input: string, close: string, start: number): number {
-  const lower = input.toLowerCase()
-  const lowerClose = close.toLowerCase()
+  const closePattern = new RegExp(escapeRegExp(close), 'gi')
   let cursor = start
   while (true) {
-    const closeIndex = lower.indexOf(lowerClose, cursor)
-    if (closeIndex === -1)
+    closePattern.lastIndex = cursor
+    const match = closePattern.exec(input)
+    if (!match)
       return -1
-    const commentIndex = input.indexOf('<!--', cursor)
-    const cdataIndex = input.indexOf('<![CDATA[', cursor)
-    const hiddenIndex = [commentIndex, cdataIndex]
-      .filter(index => index !== -1 && index < closeIndex)
+    const closeIndex = match.index
+    const window = input.slice(cursor, closeIndex)
+    const commentIndexRel = window.indexOf('<!--')
+    const cdataIndexRel = window.indexOf('<![CDATA[')
+    const hiddenIndexRel = [commentIndexRel, cdataIndexRel]
+      .filter(index => index !== -1)
       .sort((left, right) => left - right)[0]
-    if (hiddenIndex === undefined)
+    if (hiddenIndexRel === undefined)
       return closeIndex
-    const marker = hiddenIndex === commentIndex ? '-->' : ']]>'
-    const hiddenEnd = input.indexOf(marker, hiddenIndex)
+    const marker = hiddenIndexRel === commentIndexRel ? '-->' : ']]>'
+    const hiddenEnd = input.indexOf(marker, cursor + hiddenIndexRel)
     if (hiddenEnd === -1)
       return -1
     cursor = hiddenEnd + marker.length

packages/sitemapd/test/parse-perf.test.ts

@@ -0,0 +1,60 @@
+import { describe, expect, it } from 'vitest'
+import { collectSitemap } from '../src/parse'
+
+// Regression guard for a quadratic record scan. `findRecordClose` used to
+// lowercase the entire remaining buffer per call, and searched for `<!--` and
+// `<![CDATA[` with no upper bound. Ordinary sitemaps contain neither, so both
+// searches ran to the end of the document just to report "absent", once per
+// record. Parsing therefore scaled with entries x document length: a real
+// 40,000-entry, 8.8MB sitemap took about 229 seconds, which is fatal in any
+// CPU-bounded runtime (it was killing a Cloudflare Worker mid-request).
+//
+// The assertions below are deliberately generous. They are not a benchmark;
+// they only need to separate linear from quadratic, and the gap is orders of
+// magnitude.
+
+function sitemapOf(entries: number): string {
+  const urls = Array.from(
+    { length: entries },
+    (_, index) => `<url><loc>https://example.com/products/item-${index}-with-a-realistic-length-slug</loc><lastmod>2026-08-01</lastmod></url>`,
+  ).join('\n')
+  return `<?xml version="1.0" encoding="UTF-8"?>\n<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">\n${urls}\n</urlset>`
+}
+
+async function timeParse(entries: number): Promise<{ ms: number, parsed: number }> {
+  const xml = sitemapOf(entries)
+  const started = performance.now()
+  const result = await collectSitemap(xml, {
+    maxDecodedBytes: Number.MAX_SAFE_INTEGER,
+    maxEntries: entries + 1,
+  })
+  const ms = performance.now() - started
+  return {
+    ms,
+    parsed: result._tag === 'document' ? result.document.entries.length : -1,
+  }
+}
+
+describe('parse performance', () => {
+  it('parses a large sitemap in linear time', async () => {
+    const { ms, parsed } = await timeParse(8000)
+
+    expect(parsed).toBe(8000)
+    // Pre-fix this took ~8.8s; post-fix it is ~0.15s.
+    expect(ms).toBeLessThan(5000)
+  }, 30_000)
+
+  it('scales sub-quadratically as the document grows', async () => {
+    const small = await timeParse(2000)
+    const large = await timeParse(8000)
+
+    expect(small.parsed).toBe(2000)
+    expect(large.parsed).toBe(8000)
+
+    // 4x the entries. Quadratic growth would be ~16x; linear is ~4x. Allow a
+    // very loose 10x so this cannot flake on a noisy machine while still
+    // failing outright if the quadratic behaviour returns.
+    const growth = large.ms / Math.max(small.ms, 1)
+    expect(growth).toBeLessThan(10)
+  }, 60_000)
+})