Parallelize unit plots in export_report - #4757
Conversation
|
Thank a lot for this @JESUSROYETH. I'm in two minds on this one, because the export report step is not particularly long relative to the sorting itself (e.g. 1< minute after a sorting that takes 1 hour) and it only needs to run once. I'm not sure it is worth the additional complexity in the code. That being said, it is common to have hundreds of units with 384 channels over say 10 cores - I wonder the export time in this case + the performance speed up? |
3a1f076 to
e3532a0
Compare
|
Thanks @JoeZiminski, your concern is valid. I measured the case you suggested using a generated sparse For a sorting that takes one hour, however, the combined sorting + report time only drops by about 1.70%. So the export-stage improvement is real, but your point about the total workflow is also correct. The default remains serial, although that does not remove the maintenance cost of the parallel path. This large benchmark used generated data at the requested scale, so I do not want to overstate it as a real-data end-to-end result. Given the one-time saving and the additional process-safety code, I agree the complexity question is valid. I am happy to simplify the scope, or close the PR if the team considers the maintenance cost is not justified .. |
Description
export_reportrenders each unit summary in serial, so the time grows directly with the number of units. This change uses the existing job kwargs to render those independent figures with a process pool instead. The default stays atn_jobs=1.For disk-backed analyzers, each worker loads the analyzer once, using the real format and
backend_optionsinstead of guessing from the path. This prevents abinary_folderwhose path happens to end in.zarrfrom being misdetected.Template percentiles get prepared before the pool starts, so there are no concurrent writes. Also, numerical thread pools follow
max_threads_per_worker.In-memory analyzers still run in parallel with
forkon Linux, but with a spawn process they fall back to serial because the analyzer holds weak references that can't be pickled. A disk-backed analyzer also falls back to serial when an extension is only in memory or has different parameters from the saved version: a worker reloading from disk would not see the same extension state. Read-onlybinary_folderanalyzers fall back as well. An explicitmp_context="fork"is rejected on Windows and falls back to serial on macOS.Zarr analyzers stay serial because their extension arrays would otherwise be materialized once per worker, which can multiply memory use for large dense analyzers. Saving as
binary_folderenables the parallel path without that replication.Performance
I measured the public function with numerical threads pinned to one. Baseline and candidate order alternated, and setup stayed outside the timed call.
The same 16-unit workload was also measured on 8-vCPU Intel, AMD and ARM machines:
Across the controlled comparisons, the PNG count and bit-for-bit hash stayed the same. The ARM row has two measured repetitions instead of three, so that limit is explicit here. For the 256-unit case, the export itself saved 62.462 seconds at the median; against a one-hour sorting run, that would reduce the combined sorting-plus-export wall time by about 1.70%, so the larger export-stage speedup should not be confused with the total workflow speedup.
Validation:
pytest -m exporters -q: 22 passed, 1391 deselected.pytest -m core -qwith Pillow and Matplotlib unavailable: 339 passed, 4 skipped, 1067 deselected.pytest -m exporters -q: 20 passed, 3 skipped, 1387 deselected.binary_folderanalyzer and exercise the in-memory Linuxforkfast path. They also check the Zarr, in-memory spawn, lazy/mismatched-extension and read-only fallbacks.backend_options.mainrevision and pass with this patch; the pre-existing serial test passes on both.forktests run only on Linux. Windows rejects that context, while macOS falls back to serial with an actionable warning.git diff --checkare clean.I also ran the exporter stage by hand on Windows Server 2022 after fixing two Windows-specific assumptions in the tests. I did not run macOS by hand, so project CI is still the real macOS check.
Fixes #2771.