perf(common-crawl): parse WARC files with fastwarc instead of warcio - #2373
perf(common-crawl): parse WARC files with fastwarc instead of warcio#2373chethanuk wants to merge 2 commits into
Conversation
…ecord behaviour Adds two behaviour locks around CommonCrawlWarcIterator before swapping its parser: the yielded content must be the decoded HTTP body (including Content-Encoding: gzip), and a record the parser cannot read must be logged once and end the file rather than looping. Both hold with the current warcio implementation, so they pin the contract the fastwarc swap has to preserve. Refs NVIDIA-NeMo#778 Signed-off-by: ChethanUK <chethanuk@outlook.com>
warcio parses every WARC record's headers in Python before the iterator can discard the ~2/3 that are not responses. fastwarc filters by record type in C++ first, so only response records ever reach Python. - CommonCrawlWarcIterator and both WARC readers in download.py now use fastwarc.warc.ArchiveIterator with record_types=WarcRecordType.response, which replaces the explicit rec_type check. - auto_decode="all" keeps the yielded content equal to warcio's content_stream(), which decoded the HTTP body's Content-Encoding. - strict_mode=False resynchronises past a record whose WARC header cannot be parsed. warcio abandoned the rest of the file at that point and fastwarc's default ends it silently; with strict_mode=False the records after the corrupt one are still yielded. The corrupt-record lock is rewritten as a table over a corrupt header first, mid-file, and a stream that cannot be opened at all. The skipped record itself is dropped without a log line: fastwarc 0.15.2 exposes no skip counter or callback and does not surface it even with record_types=any_type, so nothing is left to report. warcio logged the failure and then abandoned the rest of the file. - The iterator's comment records the gaps that come with fastwarc 0.x: that silent resync, chunked transfer-encoding left undecoded, and the HTTP preamble only stripped from records declaring Content-Type: application/http, which Common Crawl emits. - next() moves into its own try block: a stream fastwarc cannot open raises from next() rather than from reading a record, so it is logged once and ends the file instead of reaching the per-record "log and continue" arm. Failures while reading an individual record still log and continue, unchanged. - fastwarc<1 replaces warcio in text_cpu. fastwarc was already installed transitively by resiliparse, so no new wheel is pulled in; the two pin each other exactly, so the cap also holds resiliparse below 1.x. The cap is also load-bearing on its own: fastwarc 1.x no longer resynchronises under strict_mode=False. Verified on the pinned fastwarc 0.15.2 against 3,208 records of a real CC-MAIN-2024-10 WARC: both parsers yield byte-identical records (sha256 1abd9f30...), with fastwarc faster by something on the order of 20% on a machine that was not idle. ARC support is dropped along with arc2warc=True. Common Crawl URL generation only ever emits warc.paths.gz, so no supported input path is affected. Closes NVIDIA-NeMo#778 Signed-off-by: ChethanUK <chethanuk@outlook.com>
Greptile SummaryThis PR replaces warcio with the already-transitive fastwarc parser throughout Common Crawl WARC processing.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking request to strengthen exact-output coverage for the two range-fetch readers. The parser migration is covered for the main iterator and preserves dependency availability across supported extras; the only accepted concern is that two independently changed parsing paths could regress body extraction without their current tests detecting it. Files Needing Attention: nemo_curator/stages/text/download/common_crawl/download.py; tests/stages/text/download/common_crawl/test_warc_reader.py Important Files Changed
Sequence DiagramsequenceDiagram
participant Pipeline
participant Storage as fsspec / HTTPS / S3
participant Decoder as gzip
participant Parser as fastwarc
Pipeline->>Storage: Open WARC file or fetch byte range
Storage-->>Decoder: Compressed or raw WARC bytes
Decoder-->>Parser: WARC stream
Parser->>Parser: Filter response records
Parser->>Parser: Strip HTTP headers and decode body
Parser-->>Pipeline: URL, WARC ID, source ID, body bytes
Reviews (1): Last reviewed commit: "perf(common-crawl): parse WARC files wit..." | Re-trigger Greptile |
| ) | ||
| for record in archive_iterator: | ||
| if record.rec_type == "response": | ||
| return record.content_stream().read() | ||
| return record.reader.read() | ||
| except Exception as e: # noqa: BLE001 | ||
| logger.debug(f"Failed to parse WARC record {filename}: {e}, returning decompressed bytes") |
There was a problem hiding this comment.
The parser migration changes body extraction and decoding in both range-fetch readers, but their tests only check that the result is not None. They do not verify that the returned bytes are the exact HTTP body. Please add exact-body assertions, including a Content-Encoding case, so the tests detect regressions that return WARC or HTTP headers or compressed bytes.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Description
Problem
CommonCrawlWarcIteratorcarried a TODO pointing at #778: use fastwarcinstead of warcio. warcio parses every record's headers in Python and hands them to the iterator,
which then throws away roughly two thirds of them — the
warcinfo,requestandmetadatarecords — before doing any work. fastwarc filters by record type in C++, so only response records
ever cross into Python.
Root cause
warc_iterator.pybuiltArchiveIterator(file_pointer, arc2warc=True)and filtered withif rec.rec_type == "response". Two more warcio iterators sat indownload.py(
_fetch_from_s3and_read_warc_record), so swapping only the first would have leftwarcioimported and the dependency un-removable.
The change
fastwarc.warc.ArchiveIteratorwithrecord_types=WarcRecordType.response, which replaces the explicitrec_typecheck.auto_decode="all"keeps the yielded content equal to warcio'scontent_stream(), whichdecoded the HTTP body's
Content-Encoding. Without it a gzip-encoded body comes back as rawgzip bytes.
strict_mode=Falseat all three call sites. Without it, a record with an unparseable WARCheader ends the file: the good records after it are never yielded. warcio kept going, so the
flag is what preserves that.
warc_iterator.py,next()moves into its owntry. A stream the parser cannot open at allleaves nothing to resynchronize to, so that case is logged once and ends the file. Failures
while reading an individual record still log and continue, unchanged.
pyproject.toml:warcioout,fastwarc<1in. fastwarc was already installed transitively byresiliparse, so no new wheel is pulled in. The cap is load-bearing twice over: the twopackages pin each other exactly so it also holds resiliparse below 1.x, and fastwarc 1.x drops
the
strict_moderesync this iterator now relies on.test_mixed_record_types...fixture now emitsContent-Type: application/http;msgtype=responseon its response record, as real Common Crawl WARCs do. fastwarc only strips HTTP headers from
records that declare it.
Test evidence
Behaviour locks were added and committed first, green against warcio, so they pin the contract the
swap has to preserve: the yielded content must be the decoded HTTP body (parametrized over plain
and
Content-Encoding: gzip), and a corrupt record must not cost the rest of the file(parametrized over corrupt-header-first, corrupt-header-mid-file, and an unopenable stream).
Suite (
tests/stages/text/download/common_crawl/): 73 passed before, 78 passed after.Against a real WARC — the first 50 MiB of
CC-MAIN-2024-10/segments/1707947473347.0/warc/CC-MAIN-20240220211055-20240221001055-00000.warc.gz,3,208 response records — both parsers produce byte-identical output. Measured on the versions this
PR pins,
fastwarc 0.15.2/resiliparse 0.15.2/warcio 1.8.1, with every record's(url, warc_id, source_id, content)folded into one digest:Timing over the same file, best of three, full content read: warcio 1.14 s, fastwarc 0.84 s. The
measuring machine was not idle and the run-to-run spread is a meaningful fraction of that gap, so
read it as "roughly 20% faster here", not as a benchmark. It lands in the range of the ~25%
reported on the issue rather than the ~4x fastwarc advertises.
ruff checkandruff format --checkclean at the version.pre-commit-config.yamlpins;uv lockregenerated (Removed warcio v1.7.5,fastwarcstays at the already-locked 0.15.2).Limitations
arc2warc=True; fastwarc has no equivalent. Common Crawl URLgeneration only ever emits
warc.paths.gz, so no supported input path is affected.ChunkedDataReaderhandled it; fastwarc0.15.2 does not, even with
auto_decode="all"(1.x does). Its frequency in Common Crawl wasnot measured; the 3,208-record sample shows no divergence.
strict_mode=Falsebuys backthe records after a corrupt WARC header, but the corrupt one itself disappears silently: fastwarc
0.15.2's
ArchiveIteratorexposes no skip counter and no callback, emits no warning, and does notsurface the record even with
record_types=any_type, so there is nothing the loop can detect andlog. warcio was noisier here — it raised
ArchiveLoadFailed, which the old loop logged beforelosing the rest of the file. The trade is fewer lost records, but no signal on the one that is
lost; a short record count is the only symptom. Documented in the iterator's comment.
Content-Type: application/http.warcio inferred HTTP-ness from the record type; fastwarc requires the header. Common Crawl emits
it, so its records are unaffected, but a hand-built or non-conforming WARC response record now
yields
HTTP/1.1 200 OK\r\n...as part ofcontent.WarcRecord.parse_http()does not overridethis (it is a no-op when
is_httpis false), so matching warcio would mean reimplementing itssniffing in Python; out of scope here.
closes #778
Usage
No public API change:
CommonCrawlWarcIteratorkeeps its signature and the download stages callit exactly as before. The swap is internal to WARC parsing.
Checklist