Skip to content

orthophoto merge: parallelize the block loop (in-order writer)#2035

Open
Chouffe wants to merge 6 commits into
OpenDroneMap:masterfrom
Chouffe:parallel-orthophoto-merge-2034
Open

orthophoto merge: parallelize the block loop (in-order writer)#2035
Chouffe wants to merge 6 commits into
OpenDroneMap:masterfrom
Chouffe:parallel-orthophoto-merge-2034

Conversation

@Chouffe

@Chouffe Chouffe commented Jun 19, 2026

Copy link
Copy Markdown

Summary

Parallelizes the per-block blend loop in merge(), behind a max_workers parameter (default 1 = unchanged serial behavior), wired from stages/splitmerge.py as args.max_concurrency.

Stacked on #2036 (gated reads). This PR currently shows both commits; once #2036 merges it reduces to just the parallelization. The gating is a prerequisite — see below.

Why it needs the gated reads (#2036)

The block loop is embarrassingly parallel, but a naive thread pool stalls: rasterio's boundless=True reads serialize a VRT (_serialize_xml) on every read, which under concurrency dominates and effectively hangs the merge (workers park in _serialize_xml). #2036 replaces those with gated reads (no VRT for in-bounds windows), which makes a parallel loop viable.

Parallelization

  • Parallel compute + single in-order writer. Blocks are computed in a ThreadPoolExecutor; one thread writes them in strict block order with a bounded look-ahead (cap = 2 * max_workers). A naive "write each block as it finishes" version can't flush incrementally — GDAL needs writes in row-major (block) order — so dirty blocks accumulate in RAM until it OOMs; the in-order writer keeps writes sequential/flushable and memory small.
  • Bounded GDAL block cache during the merge (restored on exit) so dirty-tile eviction/flush under GDAL's global lock stays prompt.
  • Per-thread source handles — GDAL/rasterio datasets are not thread-safe.
  • Preserves --merge-skip-blending.

max_workers <= 1 is a plain serial compute-then-write path, byte-for-byte identical to the original loop.

Correctness

  • Deterministic across worker counts. On a real 15.9 Gpx survey (3 submodels, ~61k blocks), the merged orthophoto is byte-for-byte identical at max_workers=1 (serial, ~28 min) and max_workers=16 (~8 min) — the in-order writer makes the result independent of worker count, and the serial/default path completes cleanly with no read↔write stall.
  • End-to-end. Verified on the same survey: the merge completes with zero stalled blocks and produces a valid orthophoto, pixel-identical to the serial baseline (confirmed for both LZW-compressed and uncompressed output).

Notes

  • No new dependencies.
  • Default behavior unchanged (max_workers=1).

@Chouffe Chouffe changed the title orthophoto merge: parallelize block processing with parallel_map orthophoto merge: parallelize block processing (in-order writer + gated reads) Jun 23, 2026
@Chouffe
Chouffe force-pushed the parallel-orthophoto-merge-2034 branch from 4fdec37 to bbd0295 Compare June 23, 2026 21:34
merge() reads each source window with rasterio boundless=True, which builds an
in-memory VRT and serializes it via Python's ElementTree (_serialize_xml) on
every read — a large per-read overhead on big merges (tens of thousands of
blocks x 3 passes x N submodels).

_read_window_gated() keeps identical output but avoids the VRT for the common
cases: a plain non-boundless read when the window is fully inside the source,
zeros when fully outside (== the 0 nodata fill boundless produces there), and
boundless only for the rare partial-edge windows.

Pixel-identical (verified: hundreds of fully-in-bounds windows across a real
merge grid compared boundless vs plain read, 0 mismatches). Serial; no behavior
change beyond the speedup. Also a prerequisite for parallelizing the merge:
boundless's per-read VRT serialization is pathological under concurrency.
@Chouffe
Chouffe force-pushed the parallel-orthophoto-merge-2034 branch from bbd0295 to fa47f06 Compare June 23, 2026 21:56
@Chouffe Chouffe changed the title orthophoto merge: parallelize block processing (in-order writer + gated reads) orthophoto merge: parallelize the block loop (in-order writer) Jun 23, 2026
@smathermather
smathermather requested a review from spwoodcock June 25, 2026 02:16
@spwoodcock

Copy link
Copy Markdown
Member

Thanks @Chouffe - this could be a really nice change! 😁

Before I do a full review, could you help me with two things please:

  1. You say the outputs are unchanged from before. Which test dataset did you use to verify? Could you provide any logs of the successful run?

  2. I don't think ODM has a policy set for LLM-assisted PRs needing to be declared (yet). For full transparency, was every line written by you, or was this assisted?

Thanks a lot for the contribution! 🙌

@Saijin-Naib

Copy link
Copy Markdown
Contributor

Our policy, as it stands currently, id here:
https://github.com/OpenDroneMap/documents/blob/master/CONTRIBUTING.md

@spwoodcock

Copy link
Copy Markdown
Member

Our policy, as it stands currently, id here:
https://github.com/OpenDroneMap/documents/blob/master/CONTRIBUTING.md

Thank you @Saijin-Naib! 🙏

I might make a little PR to ensure thats linked in the pull request template, so devs are definitely made aware when they create a new PR =)

@Chouffe

Chouffe commented Jul 6, 2026

Copy link
Copy Markdown
Author

@spwoodcock thanks for taking the time to review this PR!

For context: we’re using ODM on large drone surveys and letting users upload flight data to an online platform. We’ve already tuned ODM for our bird-monitoring domain so it runs reliably on AWS ECS Fargate. Even so, we hit one remaining issue: a large stress-test survey would deadlock after ~7 hours of compute.

To address this, I ran ODM from a pinned base Docker image (v3.6.0) with the relevant runtime patches baked in. With these changes, our very large surveys complete successfully without the earlier hiccups.

After extended investigation, the deadlock turned out to be caused by a combination of compounding factors:

  1. rasterio “boundless” mode becoming pathologically slow as the GDAL cache fills (including occasional read stalls). The fix is in a companion ODM PR: orthophoto merge: avoid boundless-read VRT serialization (gated reads) #2036

  2. The merge step running single-threaded, which is a significant bottleneck when multiple cores are available. We can speed it up with parallelism, while still ensuring GDAL writes blocks in the correct order to avoid write stalls.

To validate the merge changes, I reproduced the merge path in isolation by generating three orthophotos (via ODM’s split phase) and then running the merge step across those three outputs. I also confirmed the merged results are byte-identical whether run serially or with a high worker count.

The sub-orthophotos are large (≈5–10GB each), so if you’d like to verify that the read stalls occur in the problematic scenario, tell me what’s the best way to share artifacts with you.

Finally, this work was supported by an LLM, and the investigation itself took about a week of iterative runs to reproduce the exact prod behavior.

@spwoodcock spwoodcock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is really nice - I only left two comments, for the same line of code - hopefully useful 😄

Comment thread opendm/orthophoto.py Outdated
@Chouffe

Chouffe commented Jul 8, 2026

Copy link
Copy Markdown
Author

Thanks for the review @spwoodcock, both are good catches. Addressed in the latest push:

1. File-handle bound. Agreed on the EMFILE risk now that split-merge passes max_concurrency by default. I went with your hard-cap suggestion rather than an RLIMIT_NOFILE probe, for the cross-platform reason you gave:

MERGE_WORKER_CAP = 16

if max_workers > MERGE_WORKER_CAP:
    log.ODM_INFO("Capping orthophoto merge workers %d -> %d to limit open file handles"
                 % (max_workers, MERGE_WORKER_CAP))
    max_workers = MERGE_WORKER_CAP

The cap is applied once inside merge(), so it protects every caller. This is a pure concurrency bound, so it doesn't change the output (blocks are computed independently and written in strict order regardless of worker count) and the byte-identical guarantee still holds.

2. Skip cut opens in skip-blending mode. Done. The cut rasters are only read in the third (blend) pass, which is skipped in that mode, so there's no reason to open them:

srcs = [(rasterio.open(o), None if merge_skip_blending else rasterio.open(c))
        for o, c in inputs]

The cleanup loop now guards the None handle.

Chouffe added 5 commits July 8, 2026 11:35
Assert the gated read is pixel-identical to a boundless=True read across all
three branches (fully inside, fully outside each edge, partial-edge overlap),
which is the correctness property the gated-read optimization relies on.
Runs under the ODM image's rasterio/GDAL: python3 -m unittest tests.test_orthophoto.
…orkers)

With boundless reads gated (previous commit), parallelize the per-block blend
loop. Blocks are computed in a ThreadPoolExecutor and written from a single
thread in strict block order with a bounded look-ahead (cap = 2 * max_workers),
so writes to the compressed, tiled GeoTIFF stay sequential and incrementally
flushable and memory stays small. GDAL's block cache is bounded during the merge
(restored on exit). Per-thread source handles (GDAL/rasterio datasets are not
thread-safe). Preserves --merge-skip-blending.

Wired from stages/splitmerge.py as max_workers=args.max_concurrency.
max_workers<=1 is byte-for-byte identical to the original serial loop.
Address review feedback on the parallel merge:

- Hard-cap merge worker threads at MERGE_WORKER_CAP=16. Each worker opens
  every input pair, so handles scale as 2 * pairs * max_workers; with many
  submodels on a high-core machine (split-merge now passes max_concurrency)
  this could exhaust RLIMIT_NOFILE (EMFILE). A fixed cap keeps a meaningful
  speedup and is more portable than probing RLIMIT_NOFILE (absent on Windows).
- Skip opening the cut rasters in --merge-skip-blending mode (they are never
  read), and guard their cleanup for the resulting None handle.
Re-stacking the parallel merge onto the updated gated-reads base (which defines
_read_window_gated right before merge()) left a second copy of that helper at the
top of the file; remove it and keep the canonical one. Also replace the remaining
em-dashes in the bounded-cache / block-loop comments with plain punctuation.
@Chouffe

Chouffe commented Jul 15, 2026

Copy link
Copy Markdown
Author

@spwoodcock let me know if there is anything else I can do for this PR to get merged.

@spwoodcock spwoodcock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good to merge imo! Thanks again 🎉

@Chouffe

Chouffe commented Jul 20, 2026

Copy link
Copy Markdown
Author

Great to hear! I can't merge myself as I don't have the permissions on this repo. Looking forward to seeing this change land in ODM 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants