Skip to content

perf(service): parallelize intra-task tile merge (MAPCO-11325) - #260

Open
shimoncohen wants to merge 3 commits into
masterfrom
svc-1-parallel-merge
Open

shimoncohen wants to merge 3 commits into
masterfrom
svc-1-parallel-merge

Conversation

@shimoncohen

@shimoncohen shimoncohen commented Aug 4, 2026 •

Copy link
Copy Markdown
Collaborator

SVC-1 of MAPCO-11318. Stacked on #255 (MAPCO-11320) — base branch is logic-1-gpkgclient-perf, not master, so this diff shows only the SVC-1 change. Retarget to master after #255 merges.

Change

The service merged tiles sequentially in a nested x/y loop. Merge is the hot path and each coord is independent, so:

  • Enumerate each batch's coords and process them in chunks of batchMaxSize, merging each chunk's coords with Parallel.ForEach into a ConcurrentBag, then flushing the chunk to the target before starting the next.
  • Memory stays bounded to ~one chunk (count-based). With the default batchMaxSize=1000, that's ≈ the previous 20 MB byte cap for jpeg tiles. Per the design decision, the precise byte cap is approximated by the count cap.
  • Concurrency safety: within a chunk the target is only read (the single write happens after the parallel phase), and GetCorrespondingTile is thread-safe per data source — so this relies on the thread-safe GpkgClient connection from perf(gpkg): connection reuse + prepared bulk-insert + keyset paging (MAPCO-11320) #255.
  • DOP is configurable via GENERAL:parallel:numOfThreads (0/unset → Environment.ProcessorCount).

Scope — delivers P9 of MAPCO-11325 only

MAPCO-11325 scopes four points; this PR implements P9 (parallelize the intra-task merge). The rest remain open in that ticket and are intentionally out of scope here:

  • P17 — whole-task dispatch concurrency (MainRunner: one task at a time process-wide → per-type bounded workers). Job-state/heartbeat complexity; separate change.
  • P18 — progress HTTP PUT throttling (blocking PUT every batch and every flush).
  • P15 — job-callback caching (HTTP GET per task for the callback URL).

Separately, the count-based memory cap here is an approximation of the old byte-accurate 20 MB bound; tracked as its own follow-up in MAPCO-11367.

Testing

Full suite: 1142 passed, 0 failed. The batch-flush test now uses dense bounds and asserts count-based (chunked) flushing.

Depends on

🤖 Generated with Claude Code

@shimoncohen shimoncohen self-assigned this Aug 5, 2026
@shimoncohen
shimoncohen marked this pull request as ready for review August 5, 2026 20:20
asafmas-rnd
asafmas-rnd previously approved these changes Aug 6, 2026
@asafmas-rnd

Copy link
Copy Markdown
Contributor

Check that Parallel doesn't call other parallel's

@shimoncohen

Copy link
Copy Markdown
Collaborator Author

Check that Parallel doesn't call other parallel's

Checked. There is one real nest: this PR's outer Parallel.ForEach (TaskExecutor) → GetCorrespondingTile → InternalGetLastExistingTile, which has had an unbounded Parallel.ForEachAsync + sync-over-async .Result since #119. Under the outer loop that blocks a worker thread while inner items compete for the same pool → thread-pool starvation. Correctness is fine (independent loops, connection-guarded), the hazard is perf-only and gated to the upscale-fallback path.

The pre-existing S3 InternalUpdateTiles parallel runs sequentially per chunk, outside the merge phase — not nested, no compounding.

Fixed the inner parallel in #264 (sequential highest-zoom-first, short-circuits): #264

Base automatically changed from logic-1-gpkgclient-perf to master October 15, 2026 11:19
The service merged tiles one coord at a time in a sequential x/y loop. Merge
is the hot path and each coord is independent, so run each batch's coords
through Parallel.ForEach, one chunk at a time.

- Coords are enumerated per bounds and processed in chunks of batchMaxSize.
  Each chunk is merged in parallel into a ConcurrentBag, then flushed to the
  target before the next chunk starts, so peak memory stays bounded to ~one
  chunk (count-based; with the default batchMaxSize=1000 this is ~the previous
  20MB byte cap for jpeg tiles).
- Within a chunk the target is only read (writes happen after the parallel
  phase), and GetCorrespondingTile is thread-safe per data source, so the merge
  relies on the thread-safe GpkgClient connection from MAPCO-11320.
- Degree of parallelism is configurable via GENERAL:parallel:numOfThreads
  (0/unset -> Environment.ProcessorCount).

Depends on and is stacked over MAPCO-11320 (logic-1-gpkgclient-perf).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread MergerService/Runners/TaskExecutor.cs
Comment thread MergerService/Runners/TaskExecutor.cs
shimoncohen and others added 2 commits October 21, 2026 10:47
…rvation

InternalGetLastExistingTile fetched all ancestor coords via an unbounded
Parallel.ForEachAsync and blocked on .Result. When called from inside the
service-level Parallel.ForEach merge loop this nests parallelism and, via the
sync-over-async .Result, blocks an outer worker thread while inner Task.Run
items compete for the same thread pool -> starvation under load. The inner
GetTile calls also serialize on the source connection, so the concurrency
bought little.

Replace with a sequential highest-zoom-first loop that short-circuits on the
first existing tile. Same result (closest ancestor), fewer GetTile calls, no
nesting, no sync-over-async.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Default 4 rather than 0: 0 falls back to Environment.ProcessorCount,
which on .NET 6 honors the cgroup CPU limit but returns node core count
when no limit is set.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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