fix(crc32c): accept any contiguous bytes-like object in value() and extend() by yuxin00j · Pull Request #18626 · googleapis/google-cloud-python

yuxin00j · 2026-10-12T01:58:21Z

Summary

google_crc32c.value(), extend() and Checksum.update() accept any contiguous bytes-like object (memoryview, bytearray, array, bytes) instead of bytes only, and the C extension releases the GIL for inputs of 1 MiB or more whose memory belongs to a bytes object (a bytes or a memoryview of one). That threshold is exported as google_crc32c.GIL_RELEASE_THRESHOLD (1 MiB).

Motivation: the storage client wants to verify chunks it receives as memoryview slices of the gRPC message without copying them into bytes first; today value(memoryview(...)) raises TypeError: argument 1 must be read-only bytes-like object.

Changes

src/google_crc32c/_crc32c.c: parse the data argument with y* into a Py_buffer (released after hashing) instead of y#, which rejects every exporter with a release function. The 1 MiB GIL release applies when the exported memory is owned by a bytes object; a bytearray or other writable owner keeps the GIL, since another thread could write to it mid-hash.
src/google_crc32c/python.py: the pure-Python implementation hashes the raw bytes of the same inputs (a multi-byte array or cast view is hashed byte for byte, as the C extension does, instead of item by item). Checksum(initial_value) no longer compares the initial value with b"", which raised for numpy arrays.
GIL_RELEASE_THRESHOLD (1 MiB) is exported from _crc32c.c, cext.py, python.py and the package, so callers such as the storage client (perf(storage): verify CRC32C of large read chunks off the event-loop thread #18628) can size the chunks they hash off the event loop.
Docstrings in cext.py, _checksum.py and python.py say bytes-like.
tests/test___init__.py: value/extend on bytes, bytearray, memoryview (sliced, writable, read-only, cast, 2-D) and array inputs, BufferError on non-contiguous views, and GIL probes: held for bytearray and read-only views of it, released for bytes, memoryview(bytes) and a slice of it.

Tests

nox -s unit lint cover

184 passed, 4 skipped (numpy not installed) with the compiled extension; 89 passed, 99 skipped, 100% coverage in pure-Python mode (nox -s unit cover).

Notes

Behavior change in the pure-Python implementation for multi-byte array/cast inputs (previously coerced item by item). It now matches the C extension.
The GIL rule (bytes-owned memory only) is a choice: hashlib and zlib.crc32 release the GIL for any buffer. Holding it for writable owners avoids hashing memory another thread may be writing to; happy to drop that restriction if you prefer parity with hashlib.

…xtend() `google_crc32c.value()`, `extend()` and `Checksum.update()` parsed their chunk with `PyArg_ParseTuple("y#")`, which only accepts objects without a `releasebuffer` slot, so `memoryview`, `bytearray` and `array.array` were rejected with "argument must be read-only bytes-like object". Callers that hold a view into a larger buffer had to copy it into `bytes` first just to checksum it. Parse the chunk as a `Py_buffer` ("y*") instead, which accepts any C-contiguous object supporting the buffer protocol, and release it once the checksum is computed. The GIL is still released for chunks of at least 1 MiB, now for any read-only buffer (`bytes`, or a read-only view) rather than only `bytes`; mutable buffers such as `bytearray` keep the GIL as before. Non-contiguous views raise `BufferError`, as with `hashlib`.

Add a non-zero-offset memoryview slice and a writable bytearray-backed view to the bytes-like parametrization so the extension is shown to hash the exported slice rather than the exporter's whole buffer, test the Checksum constructor with every bytes-like type, check that extend() rejects non-contiguous views like value() does, and align the _checksum.py / python.py docstrings with cext.py.

Checksum.update() converted its input with array.array("B", data), which iterates items rather than bytes. Multi-byte buffers such as array("H") or memoryview.cast("I") therefore hashed differently from the C extension (or raised OverflowError), and mmap, 'c'-format and N-D buffers raised, although the C extension hashes their raw bytes. Hash memoryview(data).cast("B") for buffer objects, raise BufferError for views that are not C-contiguous as the C extension does, and keep array.array("B", data) only for iterables of ints.

The extension released the GIL for any read-only chunk of at least 1 MiB, but a read-only buffer is not necessarily immutable: the owner of memoryview(bytearray).toreadonly(), or of a numpy array with writeable=False, can still write to it from another thread, so the checksum could cover a mix of old and new contents. Release the GIL only when the memory belongs to a bytes object, passed directly or through a memoryview (including slices) of it, so slices of large bytes payloads are still hashed without the GIL. All other buffers are hashed with the GIL held, as bytearray already was.

The tests proved only the negative half of the GIL rule: value() keeps the GIL for a bytearray and for read-only views of one. The property the storage SDK's off-loop checksum depends on, that hashing a bytes object or a memoryview into one of 1 MiB or more runs with the GIL released, had no test. The same probe thread now checks that it runs while value() is in progress for bytes, a memoryview of bytes and a slice of that view. The numpy-only line of the Fortran-order helper is excluded from coverage; the unit session does not install numpy, so it skips before that line and the cover session would otherwise stop at 99%. Test plan: with the extension built from this worktree (google_crc32c.implementation == "c") and with CRC32C_PURE_PYTHON=1: python -m pytest packages/google-crc32c/tests --cov=google_crc32c --cov=tests \ --cov-config=.coveragerc --cov-report=term-missing 180 pass, 4 skipped (numpy), coverage 100%; the three new cases pass on three consecutive runs. ruff check/format (0.14.14) and flake8 are clean. Authored with an AI assistant.

gemini-code-assist

Code Review

This pull request adds support for bytes-like objects (such as bytearray, memoryview, array.array, and mmap) in both the C extension and pure-Python implementations of the CRC32C checksum. It updates argument parsing to accept any C-contiguous object supporting the buffer protocol and refactors GIL release logic to safely verify buffer immutability. Additionally, comprehensive tests are introduced to validate these changes. Feedback on the C extension suggests using a recursive loop to traverse nested memoryview wrappers to ensure the GIL is correctly released for multi-layered views.

chandra-siri

@yuxin00j , nothing blocking from my side. however added a few comments please address them before merging.

yuxin00j · 2026-10-21T06:19:52Z

/gcbrun

…thread (#18628) ## Summary Hash read chunks of 1 MiB or more on a small worker pool instead of the asyncio event-loop thread, and write each chunk to its buffer only after its checksum matches. On `main`, `google_crc32c.value()` is 10-12% of the event-loop thread's samples at 2 GB/s per process (py-spy, 1 process x 16 streams), time during which no other stream on that loop is served. `google-crc32c` 1.9.0 and later release the GIL while hashing `bytes` of 1 MiB or more, so the workers run in parallel with the loop thread; this PR raises the minimum `google-crc32c` to 1.9.0. ## Changes - **`asyncio/retry/reads_resumption_strategy.py`:** - Chunks of at least `_CRC32C_OFFLOAD_MIN_BYTES` (defaults to `getattr(google_crc32c, "GIL_RELEASE_THRESHOLD", 1024 * 1024)`; configurable via `GCS_PYTHON_CLIENT_CRC32C_OFFLOAD_MIN_BYTES`, where `0` verifies every chunk inline) that are not part of a full-object read are hashed on a 2-thread `ThreadPoolExecutor`; smaller chunks and full-object reads (which feed the rolling checksum) are hashed inline as before. - Offloaded chunks (and any chunks arriving behind one) are held in `state["pending_checksums"]` and written in arrival order once their checksum matches (`verify_pending_checksums`). `next_expected_offset` advances on receipt and `bytes_written` only on write, so a retry resumes right after the last verified byte. - On a chunk checksum mismatch (which raises `DataCorruption`, as inline verification does) or an exception on the worker (which is re-raised), `discard_pending_checksums` drops every held chunk, cancels queued hashes and rewinds each read to its written end. `recover_state_on_failure` verifies held chunks before a stream is reopened. - The pool is replaced in forked children (`os.register_at_fork`); once interpreter shutdown refuses new work, chunks are hashed inline. - **`setup.py`, `testing/constraints-3.10.txt`, `storage-integration.yaml`:** require `google-crc32c >= 1.9.0`, the first release that hashes large `bytes` without holding the GIL. - **`asyncio/async_multi_range_downloader.py`:** `download_ranges` writes verified chunks after each received message (holding at most `_CRC32C_MAX_PENDING = 32`), drains fully when an attempt ends or fails with a stream error, discards held chunks on cancellation or `DataCorruption`, and documents all of this. ## Usage Enabled automatically for non-full-object read chunks of at least 1 MiB (`getattr(google_crc32c, "GIL_RELEASE_THRESHOLD", 1024 * 1024)`). Set `GCS_PYTHON_CLIENT_CRC32C_OFFLOAD_MIN_BYTES` to override the threshold in bytes, or `0` to verify every chunk inline on the event-loop thread. ## Memory `download_ranges` bounds `pending_checksums` to `_CRC32C_MAX_PENDING = 32` chunks per call before draining on the event loop. For 2 MiB chunks, each held `_PendingChunk` retains both the `data` slice and the enclosing `BidiReadObjectResponse`, bounding pending memory per `download_ranges` call to at most ~128 MiB with the generated `proto-plus`/`upb` parser (~4 MiB per chunk across the `upb` arena copy and `.content` `bytes` copy) or ~64 MiB with the zero-copy parser in #18627 (~2 MiB per chunk sharing the gRPC buffer). ## Tests `tests/unit/asyncio/retry/test_reads_resumption_strategy.py` and `tests/unit/asyncio/test_async_multi_range_downloader.py`: offload threshold, zero threshold and full-object exclusion; in-order writes behind a pending chunk; mismatch on an offloaded chunk (nothing from it on is written, offsets rewound, the retry requests the bytes again); a worker exception (queue emptied, queued futures cancelled); drain before a retry and at the end of an attempt; discard on cancellation; fork and shutdown behavior of the pool. ``` nox -s unit lint cover ``` ## Benchmark Setup: GCE `c4-standard-192`, Rapid bucket in the same zone, 8 processes x 16 concurrent `AsyncMultiRangeDownloader` streams (128 streams total), `4 x 16 MiB` ranges per `download_ranges()` call into preallocated buffers, 5 s warmup + 15 s timed window per round, 8 interleaved rounds per arm. ### 1. Off-Loop CRC32C Only vs. `main` (Neutral Alone) | Arm | Mean (GB/s) | Median (GB/s) | Min - Max (GB/s) | vs. `main` (Mean / Median) | | :--- | :---: | :---: | :---: | :---: | | `main` (generated parser + inline CRC32C) | 9.62 | 9.83 | 8.25 - 10.70 | - | | `offloop` only (`GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=0`) | 9.64 | 9.96 | 7.32 - 10.85 | +0.2% / +1.3% | With the generated `proto-plus`/`upb` parser active, each 2 MiB chunk is still copied twice on the event-loop thread under the GIL (~254 us per message), so the loop remains saturated and offloading CRC32C alone is neutral. ### 2. Zero-Copy Only vs. Zero-Copy + Off-Loop CRC32C (Additive on Top of #18627) | Arm | Mean (GB/s) | Median (GB/s) | Min - Max (GB/s) | vs. `main` (Mean / Median) | vs. Zero-Copy Only (Mean / Median) | | :--- | :---: | :---: | :---: | :---: | :---: | | `main` (generated parser + inline CRC32C) | 9.47 | 9.48 | 8.07 - 10.51 | - | - | | `zero-copy` only (#18626 + #18627, inline CRC32C) | 11.24 | 11.07 | 9.62 - 13.73 | +18.6% / +16.7% | - | | `zero-copy + offloop` (#18626 + #18627 + this PR) | 13.30 | 12.96 | 10.35 - 16.02 | +40.4% / +36.7% | +18.3% / +17.1% | Once #18627 removes the two `upb`/`proto-plus` copies from the event-loop thread (254 us -> 4.7 us per 2 MiB message, +18.6% mean / +16.7% median over `main`), synchronous `google_crc32c.value()` (~100-200 us per 2 MiB chunk) becomes the largest remaining per-message work on the loop thread; offloading >= 1 MiB chunks onto the worker pool adds another +18.3% mean / +17.1% median (+40.4% mean / +36.7% median total over `main`).

…ync reads (#18627) ## Summary An opt-in parser for `BidiReadObjectResponse` data messages that reads each message in place over the `bytes` object gRPC hands to the deserializer, instead of copying every chunk twice on the event-loop thread (`upb` arena decode, then `.content` into a new `bytes`; 257 us per 2 MiB message vs. 5 us with this parser). Off by default; set `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=1` (or `true`/`yes`/`on`) to enable it. - Existing callers passing a `.write()` destination (`io.BytesIO`, binary files, custom streams) keep receiving `bytes`. A destination receives the chunk as a `memoryview` of the received message only when both the parser is enabled and the caller passes `download_ranges(..., accepts_memoryview=True)`; either switch alone leaves it `bytes`. - Callers passing a writable buffer (`bytearray` or a writable `memoryview` slice) as the destination get each chunk copied straight into it, without an intermediate `bytes` when the parser is enabled and the installed `google-crc32c` accepts `memoryview`. - Works with any `google-crc32c`: when the installed release accepts `memoryview` (#18626), chunks are views of the wire buffer; with `google-crc32c <= 1.9.0` they are `bytes` slices, which still saves the `upb` arena copy. - When opted in via `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=1`, `_AsyncReadObjectStream.recv()` and `DataCorruption.response` carry the duck-typed `_ZeroCopyBidiReadObjectResponse` on data messages instead of the generated `proto-plus` wrapper. ## Changes - **`asyncio/_zero_copy_bidi_read.py` (new):** - `deserialize(buf)` walks the top-level wire tags of data responses (`object_data_ranges`, `read_handle`), slices `checksummed_data.content` as `memoryview(buf)[start:i]` (or `buf[start:i]` when `google-crc32c` only takes `bytes`), and parses `ReadRange` and `BidiReadHandle` sub-messages with the generated parser. Varint, int64, tag, and length-prefix handling match `upb`. - A response carrying `metadata`, input that raises `(_Malformed, IndexError, DecodeError)`, or a buffer that is not an immutable `bytes` object is handed to `BidiReadObjectResponse.deserialize`, which returns the real message or raises its usual `DecodeError`. - `is_enabled()` reads `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ`; `is_supported()` probes both `google_crc32c.value(memoryview(...))` and `google_crc32c.Checksum().update(memoryview(...))` once per process. - `wrapped_rpc(transport)` builds and caches the transport's `BidiReadObject` callable with `deserialize` swapped in and the same error mapping and `x-goog-api-client` header as `_wrapped_methods`. - **`asyncio/async_read_object_stream.py`:** `_AsyncReadObjectStream` uses `wrapped_rpc` when enabled, the generated method otherwise. - **`asyncio/async_multi_range_downloader.py`:** - `download_ranges(..., accepts_memoryview=False)`: `.write()` destinations are wrapped in `_BytesWriter`, which converts `memoryview` chunks to `bytes`, unless the caller opts in. - Writable, C-contiguous buffer-protocol destinations (`bytes_to_read > 0`) are wrapped in `_BufferWriter`, which copies each chunk into the first `bytes_to_read` bytes of the buffer, raises `BufferError` if fewer than `bytes_to_read` bytes are returned, and releases its view when `download_ranges` exits. - **`asyncio/retry/reads_resumption_strategy.py`:** `_write_chunk` checks `bytes_written + len(data) > initial_length` before writing to `user_buffer`, so server over-delivery raises `DataCorruption` and closes the stream before touching the caller's buffer. ## Tests `tests/unit/asyncio/test_zero_copy_bidi_read.py`, `test_async_read_object_stream.py`, `test_async_multi_range_downloader.py`: parametrized equivalence against the generated parser (`_pb` / `HasField`, unknown fields, split sub-messages, over-long varints and length prefixes), malformed and fallback inputs, the environment switch and cached `value` / `Checksum.update` probe, `_BytesWriter` and `_BufferWriter` validation / retry / checksum-mismatch / full-object / view-release paths, an unpatched `google-crc32c` check, and an in-process `grpc.aio` server header parity test. 100% line and branch coverage of `_zero_copy_bidi_read.py` under both `upb` and `PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python`. ``` nox -s unit lint cover ``` ## Benchmark GCE `c4-standard-192`, Rapid bucket in the same zone, 8 processes x 16 concurrent `AsyncMultiRangeDownloader` streams, `4 x 16 MiB` ranges per `download_ranges()` call into preallocated buffers (`accepts_memoryview=True`), `google-crc32c` with #18626; 8 interleaved rounds per arm, aggregate GB/s: | Arm | Mean | Median | Min - Max | | :--- | :---: | :---: | :---: | | `main` | 9.47 | 9.48 | 8.07 - 10.51 | | this PR, parser enabled | 11.24 (+18.6%) | 11.07 (+16.7%) | 9.62 - 13.73 |

yuxin00j added 5 commits October 2, 2026 05:28

yuxin00j mentioned this pull request Oct 12, 2026

feat(storage): opt-in zero-copy BidiReadObjectResponse parsing for async reads #18627

Merged

gemini-code-assist Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread packages/google-crc32c/src/google_crc32c/_crc32c.c Resolved

yuxin00j mentioned this pull request Oct 12, 2026

storage: zero-copy BidiReadObjectResponse parsing and off-loop CRC32C verification for async multi-range reads #18629

Open

yuxin00j marked this pull request as ready for review October 12, 2026 04:03

yuxin00j requested a review from a team as a code owner October 12, 2026 04:03

yuxin00j requested a review from chandra-siri October 12, 2026 08:48

chandra-siri self-assigned this Oct 12, 2026

chandra-siri approved these changes Oct 19, 2026

View reviewed changes

chandra-siri requested changes Oct 19, 2026

View reviewed changes

Comment thread packages/google-crc32c/src/google_crc32c/_crc32c.c Resolved

feat(crc32c): export GIL_RELEASE_THRESHOLD and address review comments

d385ddc

chandra-siri approved these changes Oct 20, 2026

View reviewed changes

yuxin00j mentioned this pull request Oct 21, 2026

perf(storage): verify CRC32C of large read chunks off the event-loop thread #18628

Merged

zhixiangli reviewed Oct 23, 2026

View reviewed changes

Comment thread packages/google-crc32c/src/google_crc32c/python.py Resolved

Comment thread packages/google-crc32c/tests/test___init__.py Resolved

Comment thread packages/google-crc32c/tests/test___init__.py Resolved

fix(crc32c): call Checksum.update unconditionally and stabilize GIL test

26d4369

yuxin00j requested a review from zhixiangli October 30, 2026 08:20

yuxin00j merged commit c632e3e into googleapis:main Nov 3, 2026
119 checks passed

release-please Bot mentioned this pull request Nov 3, 2026

chore(main): release google-crc32c 1.9.1 #18504

Open

Navigation Menu

Repository navigation

fix(crc32c): accept any contiguous bytes-like object in value() and extend() - #18626

fix(crc32c): accept any contiguous bytes-like object in value() and extend()#18626
yuxin00j merged 7 commits into
googleapis:mainfrom
yuxin00j:crc32c-buffer-protocol

yuxin00j commented Oct 12, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

chandra-siri left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuxin00j commented Oct 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yuxin00j commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Tests

Notes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

chandra-siri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuxin00j commented Oct 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yuxin00j commented Oct 12, 2026 •

edited

Loading