the decompression of manifests is parallelized (#2164), the manifest merge/rebuild/recompress is currently CPU-bound to one core.
tl;dr, if supporting 200+ array stores w/ fast commits will at some point become a priority for icechunk, some cpu concurrency on the manifest merge+recompress path will probably become needed.
I'm not advocating for implementing that now, just want to have it in mind when naming this config option. Reasonable options in my mind
- Option 1: keep the io concurrency controlled by
max_concurrent_manifest_updates. Use available_parallelism() (like 2164 does) for auto setting the cpu parallelism.
- Option 2: rename current option to
max_concurrent_manifest_update_io. Later add max_concurrent_manifest_update_cpu
What do you think?
Some profiling results I had claude do after prototyping a more cpu concurrent implementation to make sure this was real:
Setup: 64 arrays × 150k refs each (9.6M refs); commit touches 1 ref in every array → all 64 manifests rewritten. In-memory store behind LatencyStorage (read=write delay). Manifest ref-cache disabled (forces real fetch+decode per node). AMD 5600G, 6 cores/12 threads. "cores" = process CPU-time ÷ wall.
| Scenario (commit wall time) |
0 ms latency |
30 ms latency |
| pre-#2274 (concurrency 1) |
5.60 s · 1.0 cores |
11.01 s · 0.7 cores |
| #2274, c=16 (decode parallel, rest inline) |
3.72 s · 1.5 cores |
4.05 s · 1.4 cores |
| #2274, c=16 + CPU offload (merge/rebuild/encode → blocking pool) |
1.41 s · 9.1 cores |
1.67 s · 7.4 cores |
Per-step CPU breakdown for one node's manifest update (150k refs, zstd level 3):
| Step |
ms/node |
share of inline CPU |
flatbuffer rebuild (from_sorted_vec) |
29.5 |
39% |
| zstd encode (level 3) |
26.0 |
34% |
| merge-iterate (decode-side iter+filter+collect) |
18.8 |
25% |
| sort (input is nearly-sorted in the real path) |
1.2 |
2% |
| (zstd decode — already parallel via #2164) |
8.4 |
— |
Appendix: where current serial comes from
- The driver: all node futures share one tokio task — do_flush polls them with buffer_unordered(max_concurrent_nodes); sub-futures are never spawned, so any CPU between .awaits blocks every other node:
- Merge-iterate (inline) — write_manifest_with_changes decodes-iterates every old ref and filters, directly in the node future:
- Sort + flatbuffer rebuild (inline) — write_manifest_from_stream sorts and runs Manifest::from_sorted_vec on the polling task:
- zstd encode (inline despite looking async) — compress_with_header runs async-compression's ZstdEncoder over an in-memory &[u8]; read_to_end(...).await over memory never yields, so the whole encode is CPU on the polling task:
The contrast — decode is not serial — fetch_and_decode does zstd::decode_all + deserialize on spawn_blocking, gated at available_parallelism (#2164):
Originally posted by @aldenks in #2274 (comment)
the decompression of manifests is parallelized (#2164), the manifest merge/rebuild/recompress is currently CPU-bound to one core.
tl;dr, if supporting 200+ array stores w/ fast commits will at some point become a priority for icechunk, some cpu concurrency on the manifest merge+recompress path will probably become needed.
I'm not advocating for implementing that now, just want to have it in mind when naming this config option. Reasonable options in my mind
max_concurrent_manifest_updates. Useavailable_parallelism()(like 2164 does) for auto setting the cpu parallelism.max_concurrent_manifest_update_io. Later addmax_concurrent_manifest_update_cpuWhat do you think?
Some profiling results I had claude do after prototyping a more cpu concurrent implementation to make sure this was real:
Setup: 64 arrays × 150k refs each (9.6M refs); commit touches 1 ref in every array → all 64 manifests rewritten. In-memory store behind
LatencyStorage(read=write delay). Manifest ref-cache disabled (forces real fetch+decode per node). AMD 5600G, 6 cores/12 threads. "cores" = process CPU-time ÷ wall.Per-step CPU breakdown for one node's manifest update (150k refs, zstd level 3):
from_sorted_vec)Appendix: where current serial comes from
The contrast — decode is not serial — fetch_and_decode does zstd::decode_all + deserialize on spawn_blocking, gated at available_parallelism (#2164):
Originally posted by @aldenks in #2274 (comment)