Repository navigation
rs-upstream-sweep-4: crc TableBasedRegister.update upstream PR - #79
Merged
Merged
Conversation
Target Nicoretti/crc (752K dl/mo, highest-download prod-live original not yet reviewed). Pure-Python fast loop, ~18.5x wall, memory neutral, differential oracle 1365 comparisons 0 divergences. Upstream PR Nicoretti/crc#210 (open, not a draft).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rs-upstream-sweep-4 report — crc upstream PR
Target: Nicoretti/crc @
4e65ac4(v8.0.0).Fork: John-Cusack/crc,
branch
rs-upstream-perf.Status: UPSTREAM PR OPENED — Nicoretti/crc#210
(open,
draft: no, verified viagh-axi pr view).1. Target selection (registry re-verified, not cached)
Prod PyPI re-queried per lane (sweep-1 lesson): prod-live
-rustset iscrc-rust 0.1.0, strsimpy-rust 0.2.1, python-dateutil-rust 2.9.0,
markdown-it-py-rust 4.2.0, packaging-rust 26.4.0, charset-normalizer-rust
3.5.1. TestPyPI-only (404 on prod PyPI): pyyaml-rust, pyparsing-rust,
python-multipart-rust, pygments-rust. Exclusions hold (pyyaml PR
yaml/pyyaml#976 open; charset saturated/killed; packaging no-gain;
dateutil PR dateutil/dateutil#1609 open; markdown-it-py PR
executablebooks/markdown-it-py#449 open). Remaining prod-live originals not
yet reviewed: crc and strsimpy. pypistats 2026-10-14: crc 752K dl/mo vs
strsimpy 334K — target is crc. Hot units:
BasicRegister.updateper-byte path,
Byte.reversed,registerproperty; our side measured2045x crc32-64KB (Rust mirror — not shippable upstream, see ADR-041).
2. Upstream process (followed to the letter)
Upstream HEAD
4e65ac4. Read before changing anything:bug/refactoring/documentation/feature/security/blank). Contributors page
is a thanks-list, no process gates.
checks.yml): format (black/isort viainvoke check.format),mypy --strictonsrc/, pylint (fail-under = 9.0), pytest withcoverage on 3.11–3.14 × ubuntu/macos/windows.
deps, zero native files in history;
requires-python = >=3.11.3. Baseline (their commands, before any change)
venv + editable install of upstream HEAD + pytest: 80 passed, 28 subtests
passed. Baseline green. Frozen workload for proof: mirror
bench/corpora (
corpus_1m.bin,corpus_64k.bin) + render differential oracle(
diff_oracle.py: all 26 configs × registers/Calculator + edges + chunksplits + input-type matrix + subclass seam + error paths).
4. Profiling
cProfile over 1 MB CRC32 + 16×64 KB frames + CRC16 (per §5 workload, ~6 s
profiled):
Byte.reversed1.1M calls / 1.63 s self,Byte.__getitem__8.9M calls / 1.36 s self,
Byte.value10.1M calls / 0.60 s self,registerget+set 4.7M calls / 0.38 s self — ~31M tiny calls for ~1.2Minput bytes. The table lookup is cheap; the per-byte boxing around it is
the cost. Killed without building: speeding up bit-by-bit
Register(out of bounds — documented slow path), C-level
bytes.translatereverse(no gain over a 256-entry index: translate builds a second buffer).
5. The change — ~18.5x, int-level
TableBasedRegister.updatesrc/crc/_crc.py+27/-0: override looping over plain ints with hoistedwidth/shift/table/mask locals and a 256-entry
_REVERSED_BYTEtable;non-
bytesinput and_process_byte-overriding subclasses delegate tosuper().update(exact behavior including error messages, oracle-pinned).Memory (tracemalloc peak, separate runs per sweep-3 lesson): public repro
2170637 Bboth sides, byte-identical; frozen75494 Bvs75498 B(noise). Overhead disclosed in PR body: none measured; tiny inputs
dominated by the two
isinstance/seam guard checks (ns-scale per call).Correctness: fork suite 80 passed + 28 subtests (counts identical to base);
differential oracle 106 sections / 1365 comparisons / 0 divergences;
black + isort + mypy strict clean; pylint 9.50/10 (base 9.46 — patch is
cleaner than the file it touches).
6. Proof (public-materials-only repro in the PR body)
Self-contained script (stdlib + installed
crc, seed-fixed synthetic data— no internal corpora) pasted verbatim in the PR body; numbers above are
that script's own output. No tokens, no internal paths in the PR.
7. Fork diff
src/crc/_crc.py+27/-0 (TableBasedRegister.updateoverride +_REVERSED_BYTEtable). No packaging change, no Rust.the oracle, §8.2).
8. Checklist deltas (new lessons; standing list unchanged otherwise)
candidate passed 1300+ value comparisons but raised
TypeErrorwith adifferent message than base for non-
bytesinput (str,float).Both sides raise — but a message-matching downstream test would break.
Fix pattern: restrict the fast override to the documented input type
(
isinstance(data, bytes)) and delegate everything else tosuper(),making off-contract behavior identical by construction, then pin it
in the oracle (values + messages + exotic inputs: bytearray, memoryview,
int-list, generator, str, float).
sys.pathat the package parent, never the package dir, intree-comparison harnesses. A smoke test inserted
.../src/crc(thepackage directory itself) instead of
.../src;import crcthensilently resolved to the installed editable copy on both sides and the
"speedup" measured base-vs-base (1.06x). Cross-check: assert
module.__file__starts with the intended tree before trusting anynumber.