test(py): detect a held GIL, and errors h5py does not raise
test_threads_read_the_same_file passed with the GIL held. The new test_reads_release_the_gil measures the longest stall of a spinning Python thread while another reads: with py.detach removed from the read it stalled 0.062 s of a 0.064 s read and failed; with it, about 3 ms. test_errors_match_h5py now compares the result whenever h5py reads the key, instead of only checking that we raise when h5py raises, over a longer key list. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -94,6 +94,14 @@
|
|||||||
- **`ds[np.array(1)]` is an integer index**, as in h5py; a 0-d integer
|
- **`ds[np.array(1)]` is an integer index**, as in h5py; a 0-d integer
|
||||||
array went down the index-list path and raised a confusing `TypeError`.
|
array went down the index-list path and raised a confusing `TypeError`.
|
||||||
The h5py comparison keys now include 0-d arrays on every axis.
|
The h5py comparison keys now include 0-d arrays on every axis.
|
||||||
|
- **Tests that would notice a held GIL, and our extra errors.**
|
||||||
|
`test_reads_release_the_gil` times a Python thread spinning while another
|
||||||
|
reads: with the read made to hold the GIL it stalls for the whole read
|
||||||
|
(0.062 s of a 0.064 s read) and the test fails; released, its longest
|
||||||
|
stall is about 3 ms. (The existing threads test only checked values.)
|
||||||
|
`test_errors_match_h5py` now also requires that every key h5py reads
|
||||||
|
reads here too, with the same result, and covers more keys (0-d arrays,
|
||||||
|
repeated and empty lists, `()`, `...`).
|
||||||
- **CI builds and tests the Python package.** It was excluded from CI.
|
- **CI builds and tests the Python package.** It was excluded from CI.
|
||||||
`scripts/ci-test.sh` now lints `clawhdf5-py`, builds the wheel with
|
`scripts/ci-test.sh` now lints `clawhdf5-py`, builds the wheel with
|
||||||
maturin, unpacks it under `target/` and runs the pytest suite; skipped
|
maturin, unpacks it under `target/` and runs the pytest suite; skipped
|
||||||
|
|||||||
@@ -266,7 +266,10 @@ def keys_for(shape):
|
|||||||
# checked against the values written instead (test_vlen_big_endian).
|
# checked against the values written instead (test_vlen_big_endian).
|
||||||
H5PY_MISREADS = {"vlen/f8_be"}
|
H5PY_MISREADS = {"vlen/f8_be"}
|
||||||
|
|
||||||
ERROR_KEYS_1D = [slice(None, None, -1), 10**6, -(10**6), None, (0, 0, 0, 0, 0), [3, 1], (Ellipsis, Ellipsis), 1.5, "nope"]
|
ERROR_KEYS_1D = [
|
||||||
|
slice(None, None, -1), 10**6, -(10**6), None, (0, 0, 0, 0, 0), [3, 1], (Ellipsis, Ellipsis), 1.5, "nope",
|
||||||
|
np.array(1.0), np.array(True), np.array(10**6), [0, 0], [], (), Ellipsis, (0,), [-1], np.array([1, 2]),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -322,12 +325,13 @@ def test_errors_match_h5py(h5py, pair):
|
|||||||
for name in ["num/le_i4_1d", "num/be_f8_2d_gzip", "str/vlen", "num/scalar_f8"]:
|
for name in ["num/le_i4_1d", "num/be_f8_2d_gzip", "str/vlen", "num/scalar_f8"]:
|
||||||
for key in ERROR_KEYS_1D:
|
for key in ERROR_KEYS_1D:
|
||||||
try:
|
try:
|
||||||
theirs[name][key]
|
expected = theirs[name][key]
|
||||||
except Exception as e: # noqa: BLE001
|
except Exception as e: # noqa: BLE001
|
||||||
with pytest.raises(type(e)):
|
with pytest.raises(type(e)):
|
||||||
ours[name][key]
|
ours[name][key]
|
||||||
else:
|
else:
|
||||||
pass # valid for this shape; covered by the value test
|
# h5py reads it, so must we (and the same values).
|
||||||
|
assert_same(ours[name][key], expected, f"{name}[{key!r}]")
|
||||||
|
|
||||||
|
|
||||||
def test_compound_fields_match_h5py(pair):
|
def test_compound_fields_match_h5py(pair):
|
||||||
@@ -454,8 +458,7 @@ def test_only_the_selected_chunks_are_read(h5py, tmp_path):
|
|||||||
|
|
||||||
|
|
||||||
def test_threads_read_the_same_file(pair):
|
def test_threads_read_the_same_file(pair):
|
||||||
"""Reads from many threads at once (the GIL is released during each
|
"""Reads from many threads at once return exactly what h5py returns."""
|
||||||
read) return exactly what h5py returns."""
|
|
||||||
ours, theirs, _ = pair
|
ours, theirs, _ = pair
|
||||||
names = ["num/le_f8_2d_gzip", "num/i4_3d_shuffle", "str/vlen_2d_gzip", "cmp/padded_chunked", "num/be_i8_1d"]
|
names = ["num/le_f8_2d_gzip", "num/i4_3d_shuffle", "str/vlen_2d_gzip", "cmp/padded_chunked", "num/be_i8_1d"]
|
||||||
expected = {n: theirs[n][()] for n in names}
|
expected = {n: theirs[n][()] for n in names}
|
||||||
@@ -477,6 +480,55 @@ def test_threads_read_the_same_file(pair):
|
|||||||
assert not errors, errors[:3]
|
assert not errors, errors[:3]
|
||||||
|
|
||||||
|
|
||||||
|
def test_reads_release_the_gil(h5py, tmp_path):
|
||||||
|
"""While one thread is inside a long read, another Python thread keeps
|
||||||
|
running. With the GIL held for the read, the other thread would stall
|
||||||
|
for the whole read; the test measures its longest stall."""
|
||||||
|
import sys
|
||||||
|
import time
|
||||||
|
|
||||||
|
path = str(tmp_path / "gil.h5")
|
||||||
|
data = np.arange(2048 * 4096, dtype="<f4").reshape(2048, 4096)
|
||||||
|
with h5py.File(path, "w") as f:
|
||||||
|
f.create_dataset("d", data=data, chunks=(64, 4096), compression="gzip", compression_opts=1)
|
||||||
|
ds = clawhdf5.File(path, "r")["d"]
|
||||||
|
key = (slice(0, 1000), slice(None)) # under half: the uncached selection path
|
||||||
|
t0 = time.perf_counter()
|
||||||
|
ds[key]
|
||||||
|
one_read = time.perf_counter() - t0
|
||||||
|
assert one_read > 0.03, f"a read took only {one_read:.3f} s; too short to measure"
|
||||||
|
|
||||||
|
old = sys.getswitchinterval()
|
||||||
|
sys.setswitchinterval(0.001)
|
||||||
|
stop = threading.Event()
|
||||||
|
gaps = []
|
||||||
|
|
||||||
|
def spin():
|
||||||
|
last = time.perf_counter()
|
||||||
|
worst = 0.0
|
||||||
|
while not stop.is_set():
|
||||||
|
now = time.perf_counter()
|
||||||
|
worst = max(worst, now - last)
|
||||||
|
last = now
|
||||||
|
gaps.append(worst)
|
||||||
|
|
||||||
|
try:
|
||||||
|
t = threading.Thread(target=spin)
|
||||||
|
t.start()
|
||||||
|
time.sleep(0.01)
|
||||||
|
t0 = time.perf_counter()
|
||||||
|
for _ in range(2):
|
||||||
|
ds[key]
|
||||||
|
reading = time.perf_counter() - t0
|
||||||
|
stop.set()
|
||||||
|
t.join()
|
||||||
|
finally:
|
||||||
|
sys.setswitchinterval(old)
|
||||||
|
np.testing.assert_array_equal(ds[key], data[:1000])
|
||||||
|
# Held, the spinner would stall for about one read.
|
||||||
|
assert gaps[0] < one_read / 3, f"spinner stalled {gaps[0]:.3f} s during reads of {one_read:.3f} s ({reading:.3f} s)"
|
||||||
|
|
||||||
|
|
||||||
def _v4_index_fixture(h5py, path):
|
def _v4_index_fixture(h5py, path):
|
||||||
"""One 2-D dataset per v4 chunk index (HDF5 1.10+ layout, libver='latest')."""
|
"""One 2-D dataset per v4 chunk index (HDF5 1.10+ layout, libver='latest')."""
|
||||||
data = (np.arange(37 * 23, dtype="<i4") * 7 - 1000).reshape(37, 23)
|
data = (np.arange(37 * 23, dtype="<i4") * 7 - 1000).reshape(37, 23)
|
||||||
|
|||||||
Reference in New Issue
Block a user