geotiff: parallelise strip decode (#2100)

[brendancol]

Summary

• _read_strips (local) and _fetch_decode_cog_http_strips (HTTP COG) decoded strips sequentially in a Python for-loop.
• The matching tile paths already gated on _PARALLEL_DECODE_PIXEL_THRESHOLD (64K pixels) via ThreadPoolExecutor. Codec decode (deflate / zstd / LZW) releases the GIL, so threads actually overlap C-level work.
• Both strip paths now use the same collect-decode-place pattern: build a job list, ThreadPool-map decode when n_strips > 1 and strip_pixels >= threshold, place sequentially into the result.

Microbench (4-core, uint16)

shape	codec	serial	parallel	speedup
4096x4096	deflate	130 ms	34 ms	3.82x
8192x8192	deflate	531 ms	146 ms	3.63x
8192x8192	zstd	211 ms	85 ms	2.48x
4096x4096	none	25 ms	22 ms	1.14x

Test plan

• 5 new tests in test_parallel_strip_decode_2100.py (parallel/serial parity, multi-strip pool engaged, single-strip serial short-circuit, windowed cross-strip read, HTTP COG strip parity)
• pytest xrspatial/geotiff/tests/ -q: 3998 pass, 8 pre-existing failures predate this change (read_to_array is now a private attr)

Closes #2100.

[brendancol]

PR Review: geotiff: parallelise strip decode (#2100)

Blockers

None.

Suggestions

• HTTP COG strip path has only the parallel-vs-serial parity test (TestHttpStripParallelDecode::test_parallel_decode_matches_serial). The local path has four (parity, pool engagement, serial-gate short-circuit, windowed). The HTTP path's decode_in_parallel gate at xrspatial/geotiff/_reader.py:2698-2700 mirrors the local gate exactly, but a small-strip / single-strip short-circuit test for the HTTP path (analogue of test_serial_path_used_for_small_strip) would lock the gate down on that side too. Not blocking — the local-path tests already pin the gate semantics and the HTTP and local code paths share the same threshold constant.
• Planar=2 multi-band stripped TIFF is not covered by the new tests. The strip-jobs loop has a dedicated branch for planar == 2 and samples > 1 at xrspatial/geotiff/_reader.py:1949-1963 with its own band-loop iteration. The parallel pool is agnostic to planar mode, so a regression in the planar=2 ordering would still produce a wrong-shape output rather than a runtime error. A planar=2 parity test would close this gap. Not blocking — the production callers that hit _read_strips with planar=2 are exercised by sibling tests under different fixtures, so the dispatch is not bypassed.

Nits

• xrspatial/geotiff/_reader.py:1995 introduced import os as _os inline. The module already aliases import os as _os_module at line 6 and uses that name throughout (lines 78, 156, 179, 224, 244, 410, 418, 1352-1353, and the matching HTTP strip path at line 2723). [Fixed in commit 8f487b2.]
• The four local-strip tests used tempfile.TemporaryDirectory() to host the .tif fixture, which raced the cached mmap on Windows and produced PermissionError: [WinError 32] at cleanup. _FileSource.close releases the entry into _mmap_cache but keeps the mmap alive until LRU eviction, so the file handle outlives the test on Windows. [Fixed in commit 385a11a — switched to pytest's tmp_path fixture, which defers cleanup to session teardown and tolerates Windows file locks.]

What looks good

• Mirrors the existing tile-path gate one-for-one: same _PARALLEL_DECODE_PIXEL_THRESHOLD constant, same n > 1 AND pixels >= threshold rule, same min(n, cpu_count()) worker cap. A reader who learned the tile pattern reads the strip change with no extra cognitive load.
• Job-list + pool.map + serial placement keeps the output buffer race-free without locks. pool.map preserves order, so the placement loop's zip(strip_jobs, decoded_strips) (xrspatial/geotiff/_reader.py:2002-2003) and HTTP analogue (line 2729) are correct by construction.
• Sparse-strip handling (byte_counts == 0) is preserved on both paths: the job-collection loops skip them (lines 1956-57, 1972-73), and the pre-filled result array carries the sparse fill through to the output.
• Codec decode (deflate / zstd / LZW) releases the GIL inside the C extension, so the threading actually overlaps work. The microbench table (3.8x on deflate, 2.5x on zstd, 1.1x on uncompressed) is consistent with that: uncompressed barely moves because there is no GIL-released work to spread.
• The local and HTTP paths share the same threshold constant, which means a single tuning knob covers all four strip / tile combinations.
• HTTP path leaves the per-strip dimension cap (_check_dimensions) inside the decode function rather than outside the gate, so the safety check still runs whether or not the decode goes parallel. Easy thing to forget when extracting a function into a thread worker.

Checklist

• Algorithm matches reference/paper — N/A (decode kernel unchanged, only scheduling).
• All implemented backends produce consistent results — parallel-vs-serial parity tests pin this.
• NaN handling is correct — N/A (codec decode unaffected).
• Edge cases are covered by tests — single-strip serial short-circuit, multi-strip pool engagement, windowed cross-strip read, HTTP parity.
• Dask chunk boundaries handled correctly — N/A.
• No premature materialization or unnecessary copies — decoded_strips = list(pool.map(...)) is necessary to keep the placement loop deterministic.
• Benchmark exists or is not needed — microbench in PR body suffices; no benchmarks/benchmarks/ entry needed for an internal scheduling change.
• README feature matrix updated (if applicable) — not needed.
• Docstrings present and accurate — the inline comments at the gate sites cite issue #2100 and explain the GIL-release reasoning.

[brendancol]

Follow-up review on PR #2104

Re-review of commits since the previous pass: 385a11a, 8f487b2, 96b4c42, 5734fd9.

Blockers

None.

Suggestions

None.

Nits

• xrspatial/geotiff/tests/test_parallel_strip_decode_2100.py:264-275 and :292-303 and :330-341 (and the analogous setup in :201-215 and :202-215) repeat the planar=2 fixture-write and the single-strip fixture-write. A small _write_planar2_uint16 helper would dedupe. Trivial; reads fine as-is.
• pytest.importorskip("tifffile") is called inside each planar=2 test body. The sibling test_predictor2_big_endian_gpu_1517.py gates on tifffile at module level via importlib.util.find_spec. Both patterns work; the inline form keeps the dependency local to the tests that need it.

What looks good

• The Windows CI fix targets the real cause: _FileSource.close releases the entry into _mmap_cache but keeps the mmap alive until LRU eviction, so a tempfile.TemporaryDirectory.__exit__ race against the cached mmap raises PermissionError: [WinError 32] on Windows. Switching the four affected tests to tmp_path defers cleanup to pytest's session teardown, which tolerates that race. The docstring at test_parallel_decode_matches_serial:53-60 documents the rationale so a future reader does not unwind the change without understanding why.
• The _os → _os_module consistency fix at _reader.py:1995 is exactly the right scope: a one-line cleanup that aligns with the file's nine other uses of the same alias.
• test_serial_gate_engages_on_single_strip uses thread-identity spying instead of ThreadPoolExecutor patching. That is the right move on the HTTP path because read_ranges also constructs a pool when there is more than one range. For a single-strip fixture the fetch layer short-circuits at len(ranges) == 1 (_reader.py:1174-1176), so the only _decode_strip_or_tile calls that would land in a worker thread are decode-pool calls — exactly what the test wants to detect.
• TestPlanar2MultibandStripParallel adds three tests covering: full-image local parity, windowed local parity, and windowed HTTP parity. The HTTP windowed branch (_fetch_decode_cog_http_strips:2639-2664) has its own per-band loop separate from the local one, so the HTTP test is not redundant with the local windowed test.
• The np.moveaxis(arr, 0, -1) round-trip check is the right shape-aware comparison: the reader returns (H, W, samples) while tifffile.imwrite accepts (samples, H, W) for planarconfig="separate". A naive assert_array_equal(par, arr) would have masked a band-order bug.
• rowsperstrip=128 on the planar=2 fixtures forces 8 strips per band × 3 bands = 24 strips on the parallel path, enough to engage the gate without dragging out CI.
• The HTTP planar=2 test reuses the existing _start_server helper, so the new coverage adds no extra infra.

Checklist

• Algorithm matches reference/paper — N/A (scheduling change only).
• All implemented backends produce consistent results — parity tests pin local serial vs parallel and local vs HTTP across chunky and planar=2 layouts.
• NaN handling is correct — N/A.
• Edge cases are covered by tests — single-strip serial short-circuit (local + HTTP), windowed cross-strip, planar=2 full + windowed, HTTP planar=2 windowed.
• Dask chunk boundaries handled correctly — N/A (eager-only change).
• No premature materialization or unnecessary copies — pool.map result is materialised exactly once to make the placement loop deterministic.
• Benchmark exists or is not needed — microbench table in PR body suffices.
• README feature matrix updated (if applicable) — not needed.
• Docstrings present and accurate — per-test docstrings explain branch coverage and Windows-cleanup rationale where it matters.