Skip to content

Commit 6617363

Browse files
lguerardclaude
andcommitted
perf(merge): ⚡ parallelize the boundary scan and give every pool its cores
Three places were leaving work on the table. _scan_touching_pairs was serial. Each task reads its own boundary slab and produces its own pairs, so it was embarrassingly parallel already; it now runs on a thread pool sized like the rest of the merge. (Planned with the offsets work, missed at the time.) The streaming pyramid called _stream_strided_level without n_workers, so it took the signature default of 4 threads no matter what the job was granted -- 8x short on a 32-core merge. tile_process, the direct API, never got the merge rework: it still ran zarr_native_merge and *then* relabel_sequential_zarr, paying an extra full read+write of the volume plus a Python set of every label id. It now passes sequential= so the renumbering folds into the merge's own LUT, exactly as the workflow does. The library path was slower than the workflow path. Also corrects the _level_chunks docstring: the floor overrides the one-source-chunk-per-task property at deep levels, where an output chunk covers a few source chunks instead of one. Bounded by a small constant either way, which is the property that matters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 3eae081 commit 6617363

3 files changed

Lines changed: 68 additions & 35 deletions

File tree

src/patchworks/_core.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
from ._cluster import _client_is_in_process, _distributed_client
1616
from ._io import auto_empty_threshold, load_ome_zarr
1717
from ._merge import zarr_native_merge
18-
from ._relabel import relabel_sequential_zarr
1918

2019
logger = logging.getLogger(__name__)
2120

@@ -606,17 +605,19 @@ def _cleanup_stage():
606605
tempfile.mkdtemp(prefix="bb_merge_"), "merged.zarr"
607606
)
608607

608+
# sequential=True folds the contiguous renumbering into the merge's own
609+
# LUT, so it costs a np.unique over the object count rather than the extra
610+
# full read+write (plus a Python set of every id) that a separate
611+
# relabel_sequential_zarr pass would.
609612
zarr_native_merge(
610613
stage_path,
611614
"staged",
612615
_merge_out,
613616
output_component,
614617
n_workers=_nw,
615618
show_progress=progress,
619+
sequential=sequential_labels,
616620
)
617-
if sequential_labels:
618-
logger.info("Relabelling to contiguous ids…")
619-
relabel_sequential_zarr(_merge_out, output_component)
620621
_cleanup_stage()
621622

622623
merged = da.from_zarr(_merge_out, component=output_component)

src/patchworks/_merge.py

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import logging
2323
import os
2424
import tempfile
25+
from concurrent.futures import ThreadPoolExecutor
2526
from contextlib import nullcontext as _nullcontext
2627
from itertools import product as _iproduct
2728
from multiprocessing import Pool as _Pool
@@ -155,6 +156,7 @@ def _scan_touching_pairs(
155156
component: str,
156157
chunk_shape: tuple[int, ...],
157158
label_offsets: "np.ndarray | None" = None,
159+
n_workers: int = 1,
158160
) -> np.ndarray:
159161
"""Scan chunk-boundary slabs; return (N, 2) int64 array of touching pairs.
160162
@@ -178,6 +180,9 @@ def _scan_touching_pairs(
178180
come out global without the store ever being rewritten. Each read is
179181
confined to one chunk column, so both sides are a single chunk and the
180182
offsets are plain scalars.
183+
n_workers : int
184+
Threads to scan with. Each task reads its own slab, so this is
185+
embarrassingly parallel and was needlessly serial.
181186
182187
Returns
183188
-------
@@ -189,36 +194,52 @@ def _scan_touching_pairs(
189194
arr = root[component]
190195
shape = arr.shape
191196
n_per_dim = [(s + c - 1) // c for s, c in zip(shape, chunk_shape)]
192-
specs = _boundary_face_specs(shape, chunk_shape)
193-
all_pairs: list[np.ndarray] = []
194-
for ax, pos in specs:
195-
# tile the face dimensions using chunk_shape columns
197+
198+
# One task per chunk-column of one boundary face. Each reads its own slab
199+
# and produces its own pairs, so they are independent.
200+
tasks: list[tuple[int, int, tuple[int, ...]]] = []
201+
for ax, pos in _boundary_face_specs(shape, chunk_shape):
196202
face_axes = [a for a in range(arr.ndim) if a != ax]
197203
face_ranges = [range(0, shape[a], chunk_shape[a]) for a in face_axes]
198204
for offsets in _iproduct(*face_ranges):
199-
sl: list = [slice(None)] * arr.ndim
200-
sl[ax] = slice(pos - 1, pos + 1)
205+
tasks.append((ax, pos, offsets))
206+
207+
def _one(task: tuple[int, int, tuple[int, ...]]) -> "np.ndarray | None":
208+
ax, pos, offsets = task
209+
face_axes = [a for a in range(arr.ndim) if a != ax]
210+
sl: list = [slice(None)] * arr.ndim
211+
sl[ax] = slice(pos - 1, pos + 1)
212+
for a, off in zip(face_axes, offsets):
213+
sl[a] = slice(off, min(off + chunk_shape[a], shape[a]))
214+
slab = np.moveaxis(np.asarray(arr[tuple(sl)]), ax, 0)
215+
a_vals = slab[0].ravel().astype(np.int64)
216+
b_vals = slab[1].ravel().astype(np.int64)
217+
if label_offsets is not None:
218+
grid = [0] * arr.ndim
201219
for a, off in zip(face_axes, offsets):
202-
sl[a] = slice(off, min(off + chunk_shape[a], shape[a]))
203-
slab = np.moveaxis(np.asarray(arr[tuple(sl)]), ax, 0)
204-
a_vals = slab[0].ravel().astype(np.int64)
205-
b_vals = slab[1].ravel().astype(np.int64)
206-
if label_offsets is not None:
207-
grid = [0] * arr.ndim
208-
for a, off in zip(face_axes, offsets):
209-
grid[a] = off // chunk_shape[a]
210-
grid[ax] = pos // chunk_shape[ax]
211-
b_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
212-
grid[ax] -= 1 # the chunk on the near side of the boundary
213-
a_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
214-
a_vals[a_vals > 0] += label_offsets[a_idx]
215-
b_vals[b_vals > 0] += label_offsets[b_idx]
216-
mask = (a_vals > 0) & (b_vals > 0) & (a_vals != b_vals)
217-
if mask.any():
218-
pairs = np.sort(
219-
np.stack([a_vals[mask], b_vals[mask]], axis=1), axis=1
220-
)
221-
all_pairs.append(np.unique(pairs, axis=0))
220+
grid[a] = off // chunk_shape[a]
221+
grid[ax] = pos // chunk_shape[ax]
222+
b_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
223+
grid[ax] -= 1 # the chunk on the near side of the boundary
224+
a_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
225+
a_vals[a_vals > 0] += label_offsets[a_idx]
226+
b_vals[b_vals > 0] += label_offsets[b_idx]
227+
mask = (a_vals > 0) & (b_vals > 0) & (a_vals != b_vals)
228+
if not mask.any():
229+
return None
230+
pairs = np.sort(np.stack([a_vals[mask], b_vals[mask]], axis=1), axis=1)
231+
return np.unique(pairs, axis=0)
232+
233+
nw = max(1, min(n_workers, len(tasks)))
234+
if nw <= 1:
235+
results = [_one(t) for t in tasks]
236+
else:
237+
# Reads and decompression release the GIL, so threads scale here and
238+
# nothing has to be pickled across processes.
239+
with ThreadPoolExecutor(max_workers=nw) as pool:
240+
results = list(pool.map(_one, tasks))
241+
242+
all_pairs = [r for r in results if r is not None]
222243
if not all_pairs:
223244
return np.empty((0, 2), dtype=np.int64)
224245
return np.unique(np.vstack(all_pairs), axis=0)
@@ -545,7 +566,11 @@ def zarr_native_merge(
545566
n_faces = len(_boundary_face_specs(shape, chunk_shape))
546567
logger.info("zarr_native_merge: scanning %d boundary faces…", n_faces)
547568
pairs = _scan_touching_pairs(
548-
staged_path, staged_component, chunk_shape, label_offsets=offsets
569+
staged_path,
570+
staged_component,
571+
chunk_shape,
572+
label_offsets=offsets,
573+
n_workers=n_workers,
549574
)
550575
logger.info(
551576
"zarr_native_merge: %d touching pairs → building LUT", len(pairs)

src/patchworks/plugins/ome_zarr.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import numpy as np
5757
import zarr
5858

59+
from .._chunks import cpu_allocation
5960
from .._io import load_ome_zarr
6061

6162
logger = logging.getLogger(__name__)
@@ -290,12 +291,16 @@ def _level_chunks(
290291
axes: str,
291292
level_shape: tuple[int, ...],
292293
) -> tuple[int, ...]:
293-
"""Chunking for a strided level that keeps one source chunk per task.
294+
"""Chunking for a strided level that keeps the per-task read bounded.
294295
295296
Choosing ``ceil(src_chunk / stride)`` makes each output chunk the exact
296297
image of one source chunk, so a task reads one chunk and writes one chunk
297-
with nothing shared between tasks. Capped by :data:`_CHUNK_CAP` and floored
298-
so deep levels don't fragment.
298+
with nothing shared between tasks.
299+
300+
The floor can override that at deep levels, where ``ceil(src / stride)``
301+
falls below it — an output chunk then covers a few source chunks instead
302+
of one. Still bounded by a small constant, which is the property that
303+
matters; those levels are a fraction of the volume anyway.
299304
"""
300305
out = []
301306
for c, st, a, s in zip(src_chunks, strides, axes, level_shape):
@@ -621,7 +626,9 @@ def _write_pyramid(
621626
)
622627
),
623628
)
624-
_stream_strided_level(src_arr, dst_arr, strides)
629+
_stream_strided_level(
630+
src_arr, dst_arr, strides, n_workers=cpu_allocation()
631+
)
625632
scale = [base_scale[k] * (strides[k] ** i) for k in range(len(axes))]
626633
datasets.append(_dataset(str(i), scale))
627634
logger.info("pyramid level %d: shape=%s", i, next_shape)

0 commit comments

Comments
 (0)