Repository navigation
Conversation
…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.
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.
Summary
Accept any contiguous 1D buffer-protocol object (
memoryview,bytearray,array('B'),bytes) ingoogle_crc32c.value(),google_crc32c.extend(), andChecksum, and release the GIL for read-only buffers >= 1 MiB.Changes
_crc32c_extendand_crc32c_valueinpackages/google-crc32c/src/google_crc32c/_crc32c.cfromPyArg_ParseTuple(..., "y#")/"ky#"(which setPyBUF_NDand rejectmemoryviewin CPython'sgetbuffer) to"y*"/"ky*"(PyBUF_SIMPLE) into aPy_buffer, always releasing the export withPyBuffer_Release.PyBytes_Checkwithchunk->readonlyin the >= 1 MiB GIL-release check (_extend_buffer), so zero-copymemoryviewslices ofbytesalso release the GIL during hardware CRC32C computation while the heldPy_bufferexport prevents buffer resizing.cext.py,_checksum.py, andpython.pyfrombytestobytes-like, and expandtests/test___init__.pyto cover contiguousmemoryview(including non-zero-offset slices and writable views),bytearray,array('B'), >1 MiB read-only views, andBufferErroron non-contiguous views.