Skip to content

perf: b_p_update is memory-bound on a scattered 160-byte Raw read, not compute-bound (94% of repricings never reach calc_pA) #154

Description

@cjfields

Why now

The #152 thread sweep closes the thread-scaling axis: on soil ITS2, 48 → 96
threads makes run_dada slower (194 → 204 s R1, 255 → 263 s R2), with
worker-busy time rising +87% for identical work while the map wall gets worse.
The map is bandwidth-saturated; adding cores subtracts.

What is left is the serial block. At 48 threads on ITS2 R1 that is store 16.9 +
shuffle 41.8 + p_update 20.0 + bud 0.4 = 79.0 s of 193.5 s, 41% of
run_dada
— an Amdahl cap of 2.45× even if the map became free.

Every other serial phase has been through two or three rounds of work (#132,
#136, #139, #143, #147). b_p_update has had none.

The measurement

pool / read p_update run_dada share repricings ns each
ITS2 R1 (48t) 20.0 s 193.5 s 10.3% 355.9 M 56
ITS2 R2 (48t) 31.1 s 257.2 s 12.1% 565.4 M 55
soil 16S R1 (64t) 29.7 s 654.1 s 4.5% 486.3 M 61

~56 ns per repricing is compute, not memory. For contrast the store scan —
after #148 — runs at 4.9 ns/raw-visit. get_pA is over 11× more expensive per
item than a cache-line-bound scan, which says the cost is the p-value
arithmetic rather than the traversal.

That matters for the lever: unlike the map, this is not competing for the
bandwidth that is already saturated, so it should actually scale with threads.

Structure, and the one hazard

b_p_update (src/pval.rs:45) loops over clusters; for each with update_e
set, it walks the members and, per raw:

  1. computes get_pA(...) and writes b.raws[raw_idx].pthe expensive part;
  2. selects bud_min / bud_min_prior, a per-cluster reduction whose tie-break
    is explicitly first-in-scan (min p, then max reads, then lowest r) and
    documented as matching b_bud's scan order.

Each raw belongs to exactly one cluster, so the p writes are disjoint.

Do not parallelise the outer loop over clusters. It looks like the obvious
split and it fails exactly where it is needed: early in the run there are very
few clusters and cluster 0 holds nearly every raw, so outer-loop parallelism
gives ~no parallelism in precisely the early, serial-heavy phase #152 identified
as the worst. It would optimise the part of the run that is already fine.

Do not parallelise the bud selection. Its tie-break is order-dependent, and
reordering it changes which raw buds — that is an inference change, not a perf
change.

Proposed shape

Split the two halves: compute get_pA for the round's raws in parallel,
then run the existing selection serially over the results. Selection is
comparisons on values already in hand, so it should cost a few ns/item against
the 56 ns it currently guards. get_pA is independent per raw, so there is no
reduction-order or floating-point-associativity concern.

Rough sizing, to be treated as a hypothesis and not a plan: if get_pA is ~80%
of the 56 ns, then at 48 threads 20.0 s → ~4 s, i.e. ~8% of run_dada on
ITS2. Smaller on 16S (4.5% share), so this is a pool-dependent lever and ITS2 is
the place to measure it.

Gates

Related

The other open direction from the same analysis is reducing what the map
touches rather than how many threads it gets — 2-bit sequence encoding (#28)
and replacing the k-mer screen with minimizers. Those move the knee to the
right and re-enable thread scaling, which is higher leverage than this issue but
considerably more work. This one is small, self-contained, and untouched.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions