Skip to content

Engage parallel tile decode for default tile_size=256#1557

Merged
brendancol merged 1 commit into
mainfrom
issue-1551
May 11, 2026
Merged

Engage parallel tile decode for default tile_size=256#1557
brendancol merged 1 commit into
mainfrom
issue-1551

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Fixes #1551. Changes tile_pixels > 64 * 1024 to tile_pixels >= 64 * 1024 in _read_tiles, so the default tile_size=256 (256 * 256 = 65536 = exactly 64 * 1024) lands on the parallel decode path instead of falling one pixel short.

Found during a 2026-05-09 efficiency audit pass over the geotiff module.

Benchmark

1024x1024 raster at default tile_size=256 (16 tiles), DEFLATE-compressed:

path time
sequential (old, strict >) 15.7 ms
parallel (new, >=) 4.4 ms
speedup 3.56x

Larger grids scale further: 4.3x at 2048x2048, 5.3x at 4096x4096.

Test plan

  • New test_parallel_decode_default_tile_1551.py with three cases:
    • parallel engages at default tile_size=256 with multiple tiles
    • sequential stays for small (128x128) tiles below threshold
    • sequential stays for single-tile reads regardless of size
  • Full geotiff suite: 939 passed, 0 new failures (3 pre-existing matplotlib/Python 3.14 deepcopy failures in test_features.py::TestPalette are unrelated and reproduce on main)
  • Tests use monkeypatch.setattr on concurrent.futures.ThreadPoolExecutor with no shared state, so pytest-xdist runs are safe

The gate in `_read_tiles` used a strict ``tile_pixels > 64 * 1024``
comparison. At the default tile_size=256 that product is exactly
64*1024, so the strict inequality fell short by one pixel and the
sequential path ran for the most common configuration. Switching to
``>=`` lands the default tile size on the parallel path.

For 1024x1024 deflate-compressed reads at the default tile size this
is a ~3.5x speedup; at 4096x4096 it is ~5.3x.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 10, 2026
@brendancol brendancol requested a review from Copilot May 11, 2026 03:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the parallel tile decode dispatch threshold in the GeoTIFF CPU reader so the default tile_size=256 (exactly 64 * 1024 pixels) uses the parallel decode path, addressing issue #1551 and improving decode performance for common tiled GeoTIFFs.

Changes:

  • Change _read_tiles parallel-dispatch threshold from tile_pixels > 64 * 1024 to tile_pixels >= 64 * 1024.
  • Add regression tests asserting parallel dispatch engages at tile_size=256, and remains sequential below threshold or when only a single tile job exists.
  • Expand the inline comment in _read_tiles to document the inclusive boundary and the rationale (issue #1551).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
xrspatial/geotiff/_reader.py Adjusts the parallel decode gating condition to include the default 256×256 tile size and documents the rationale.
xrspatial/geotiff/tests/test_parallel_decode_default_tile_1551.py Adds regression coverage verifying dispatch behavior at the threshold boundary and for sequential-only cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@brendancol brendancol merged commit 1aac3b7 into main May 11, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parallel tile decode gate excludes the default tile_size=256

2 participants