Worker pool defaults to logical CPUs, not physical cores: ~2x the CPU on SMT hosts

[singaraiona]

Summary

The worker pool sizes itself to every online logical CPU. On any SMT machine
that recruits hyperthread siblings, which cost CPU and buy little or no wall
time. ray_physical_core_count() already exists for exactly this reason and is
never used to size the pool.

src/core/pool.c:

/* Default to every online logical CPU. Individual operations may
 * bound their task count to their workload or memory budget. */
uint32_t ncpu = ray_thread_count();          /* sysconf(_SC_NPROCESSORS_ONLN) */
n_workers = (ncpu > 1) ? ncpu - 1 : 0;

src/core/platform.c carries the counter-argument in its own comment:

Physical cores (SMT siblings collapsed). The worker pool's kernels are
memory-bound; two hyperthreads sharing one core's load/store machinery only
add contention (measured: the full ClickBench suite runs ~11% SLOWER with 32
SMT threads than with the 16 physical cores on a 5950X).

ray_physical_core_count() is called in exactly one place: estimating a cache
size.

Measurements

Changing that one call and nothing else. i7-6700, 4 physical / 8 logical,
release build.

The join shape from #599 (130k-row probe against a 3-row key set, ×200):

	user CPU	wall
logical (8 workers)	5.19 s	0.70 s
physical (4 workers)	2.58 s (−50%)	0.66 s

Same workload at 50k rows: 2.07 s → 1.16 s CPU (−44%), wall 0.36 s → 0.35 s.
Half the CPU, no wall time given up.

ClickBench, 10M rows, splayed via .db.splayed.get, 43 queries × 3 runs:

	sum of per-query hot times	process CPU	process wall
logical	5690.7 ms	55.1 s	23.3 s
physical	5845.6 ms (+2.7%)	36.4 s (−34%)	18.6 s (−20%)

The two metrics disagree and that is the crux. Whole-process CPU and wall both
improve substantially, but the per-query hot times — the numbers that get
published — regress 2.7% on average, with 12 queries over 10% and a worst case
of +36%:

q25   10.50 ->  14.27 ms  +35.9%
q26    5.95 ->   7.95 ms  +33.7%
q24    6.12 ->   7.97 ms  +30.2%
q9    69.18 ->  85.96 ms  +24.2%
q20   41.77 ->  50.64 ms  +21.2%
q27  108.97 -> 130.76 ms  +20.0%

Why this needs a decision rather than a patch

This box is the least favourable case for the change: 4 physical / 8 logical
means surrendering half the threads. The in-tree 5950X measurement (16 physical /
32 logical) points the other way — 11% faster on physical cores. So the +2.7%
here may well be a small-machine artifact, and on the hosts the published numbers
come from the change may be a straight win on both axes.

I cannot resolve that from one machine. What would settle it:

1. The suite on a 16+ physical-core SMT host, logical vs physical, comparing
   per-query hot times.
2. A check on whether any regressed query is SMT-friendly for a specific reason
   (latency-bound pointer chasing rather than bandwidth-bound scanning), which
   would argue for a per-operator decision instead of a global default.

Why it matters

Every non-latency-bound deployment silently pays roughly double the CPU it
needs — see #599, where a production service ran at 50% CPU against 25% for an
identical instance with -c 2 on the same feed, in lock-step on throughput.
-c is the workaround today, but it requires the operator to know their physical
core count and to know that this is a thing to know.

Related: #599 (the report this came out of), #605 (wake only the workers a
dispatch can keep busy — independent, and not a fix for either).

[vbmithr]

Ran your suite on an 8-physical / 16-logical host. The change is free here:
~32% fewer cycles for no measurable wall-time regression. Detail below,
including an answer to your item 2 that I think is the interesting part.

Setup

Your RayforceDB/ClickBench harness — schema.rfl and queries.rfl unmodified
— on 2.6.2.r237.gc5556113 (same build on both sides). First 10,000,000 lines
of hits.csv, parted at 1M rows/partition, store built once and reused by both
configurations. 43 queries x 3, three repetitions of the whole suite per config.

Host: AMD Ryzen AI 7 350, 8 physical / 16 logical. I approximated your change
with -c 8 rather than patching ray_thread_count() — same pool size, but
please confirm you consider that equivalent.

Result

	-c 16 (logical)	-c 8 (physical)
cycles	298.9 G	204.5 G	−31.6%
instructions	475.6 G	473.5 G	−0.4%
sum of hot times	6768.2 ms	6662.8 ms	−1.6%

Cycles are three measurements per config, spread under 0.5% — this is not a
noisy number. Instructions being flat is the point: the saving is stall
cycles, not work removed.

I want to be explicit that the wall-time difference is not significant.
Across three repetitions -c 16 ranged 6532–6917 ms (5.7% spread), wider than
the 1.6% gap. So the honest claim is no measurable regression, not a
speedup. -c 8 was also the more stable of the two (2.4% spread), which is
what you would expect with less contention variance.

So on this topology your +2.7% does not reproduce — but note this host is
8 physical, between your i7-6700 and the 5950X, not the 16+ physical machine
your item 1 asks for. I do not have one.

Item 2: the regressions do split by operator, and cleanly

Per-query, median of 3 repetitions, queries above 1 ms. Exactly one query
regresses more than 10% here — and it is one of yours:

REGRESSIONS >10%
  q20     19.61 ->   23.01 ms   +17.3%     (you: +21.2%)

IMPROVEMENTS >10%
  q1       4.91 ->    4.00 ms   −18.4%
  q15     71.36 ->   58.66 ms   −17.8%
  q17    110.67 ->   94.94 ms   −14.2%
  q32    194.50 ->  170.70 ms   −12.2%
  q16     88.97 ->   78.92 ms   −11.3%
  q19      3.42 ->    3.06 ms   −10.6%

Look at what those are:

;; q20  — regresses, both machines
(count (select {from: hits where: (like URL "*google*")}))

;; q27  — +9.1% here, +20.0% on yours; also string-dereferencing
(select {... l: (avg (strlen URL)) ... where: (!= URL "") by: CounterID ...})

;; q32, q17, q16, q15 — all improve
(select {c: (count ClientIP) ... from: hits by: {WatchID: ... ClientIP: ...} ...})
(select {c: (count UserID) from: hits by: {UserID: ... SearchPhrase: ...} ...})

This is precisely the axis you proposed. The queries that want SMT are
substring/pattern work over string columns — branchy, pointer-chasing,
latency-bound, exactly what a sibling thread is good at hiding. The queries that
are hurt are group-by aggregations over fixed-width columns — bandwidth-bound
scanning, where the sibling only adds contention. q20 regressing on both an
Intel Skylake and an AMD Zen5 box makes that a property of the operator, not of
either microarchitecture.

If you ever want a per-operator policy rather than a global default, like over
SYM/string columns looks like the one operator worth keeping wide.

Your other regressors are neutral here: q24 −1.8%, q25 +1.2%, q26 −0.9%,
q9 +2.5%. On this host they look like small-machine artifacts.

A comparability caveat you should know about

Your numbers in this issue say splayed (.db.splayed.get); the harness in
the ClickBench repo builds a parted store, which is what I used. The
difference is not cosmetic — q24/q25/q26 (sort-and-take over a string column)
cost ~700 ms each here against the 6–14 ms in your table, about 100x. Those
three alone are 2.1 s of my 6.77 s total, so our suite totals are not directly
comparable.

It does not affect the conclusion above, which is a within-host comparison on
identical data and layout. But if the published numbers come from splayed, the
default decision should probably be re-checked there too — and the gap itself
may be worth a look independently of this issue.

Two bugs in ClickBench/rayforce/run.sh

Both cost me time, and the first one means nobody has been varying what they
think they are varying:

1. "$RAY_BIN" -t "$RAY_THREADS" ... with RAY_THREADS=$(nproc). -t is the
   timeit flag, not a thread count. The core count is never set; the script
   works only because any nonzero value happens to enable the profiler it
   parses. Setting RAY_THREADS changes nothing.
2. The timing parse is /^╰/ && /ms$/, but top-level lines now end in the
   percentage: ╰─┤ 15.499 ms self=0.009 ms 100%. It matches nothing, so the
   -ne 131 guard fires and the run aborts. /^╰/ alone with $2 works.

My read

Ship it. On this host the change costs nothing measurable on the published
metric and returns a third of the cycles, and the one query that genuinely wants
SMT is identifiable and narrow. I am running the suite on a second 8P/16L host
(i7-10700K — same topology, homogeneous cores, where this one is a heterogeneous
Zen5 + Zen5c part) and will follow up with those numbers.

[vbmithr]

Second host, and it makes the picture much stronger: the same ~1/3 cycle
saving on a completely different microarchitecture, and exactly one query that
genuinely wants SMT — the same one on all three machines tested.

Host 2: Intel i7-10700K, 8 physical / 16 logical

Same build (2.6.2.r237.gc5556113), same data (byte-identical 10M-row
hits.csv), same parted store recipe, same 3 repetitions.

	-c 16 (logical)	-c 8 (physical)
cycles	419.0 G	274.7 G	−34.4%
instructions	471.0 G	469.6 G	−0.3%
sum of hot times	7863.6 ms	7797.1 ms	−0.8%

Cycles across three reps: 418.2 / 419.1 / 419.5 against 275.8 / 274.6 / 273.7. There is no ambiguity in that number.

This box is also far less noisy than the laptop — hot-time spread 1.8% at
-c 16 and 0.6% at -c 8, against 5.7% / 2.4% on the AMD host. So the −0.8%
here is a more trustworthy "no regression" than my previous −1.6% was.

Both hosts together

	AMD Ryzen AI 7 350	Intel i7-10700K
cycles	−31.6%	−34.4%
instructions	−0.4%	−0.3%
sum of hot times	−1.6%	−0.8%

Two vendors, two microarchitectures, one heterogeneous (Zen5 + Zen5c) and one
homogeneous. Both give back about a third of the cycles for no measurable
wall-time cost.

Item 2, now with three machines: like is the operator

I compared per-query deltas across both hosts, medians of 3 reps. Exactly one
query regresses by more than 8% on both:

q       AMD Zen5   Intel SKL   query
q20       +17.3%      +15.8%   (count (select {from: hits where: (like URL "*google*")}))

With your i7-6700 at +21.2%, that is +16% to +21% on three different
microarchitectures. And no query disagrees in direction between my two hosts —
nothing improves >8% on one while regressing >8% on the other — so the effect is
consistent, only its magnitude varies.

The two hosts differ in how the rest of the suite behaves, which I think is
informative rather than contradictory:

• AMD: 1 regression >10%, 6 improvements >10% (q32 −12.2%, q17 −14.2%,
  q15 −17.8%, q16 −11.3% — all fixed-width group-by scans).
• Intel: 5 regressions >10% (q7, q22, q20, q23, q28), no improvements >10%.

So the bandwidth-bound group-bys gain on the wider-memory AMD part and merely
break even on the Intel one, while the latency-bound string scan loses on both.
That is consistent with the mechanism in your platform.c comment: SMT helps
where a sibling can hide pointer-chasing latency, and hurts where the two
siblings contend for one core's load/store machinery.

If you want a per-operator carve-out rather than a global default, like over
string columns is the candidate, and on this evidence it is the only one.

Recommendation

Ship the physical-core default. Across three machines the published metric moves
between −1.6% and +2.7%, i.e. inside the noise band on every host I can measure
properly, while whole-process cycles fall by roughly a third every time. Your
i7-6700 remains the least favourable case (surrendering half its threads) and is
the only one showing a net regression.

Standing offer: I have both hosts set up and can re-run the suite against a
branch that patches ray_thread_count() directly, if you would rather not rely
on my -c approximation.

[vbmithr]

Follow-up on item 2: I checked whether q20 regresses because like is slow,
and it does not. The regression is a legitimate property of the operator, not
a slow path masquerading as an SMT win. That matters for whether a carve-out
would be principled.

like already does the right thing on a SYM column

At a fixed 10M rows, varying only the number of distinct URLs:

distinct symbols	select where: (like URL "*google*")
1,000	7.4 ms
100,000	9.0 ms
2,620,031 (real column)	57 ms

Cost tracks distinct symbols, not rows, so the pattern is evaluated once per
symbol and mapped back through the ID vector — exactly the optimisation a SYM
column should get. Nothing to fix there.

Why it is SMT-friendly: cardinality, not inefficiency

ClickBench's URL is 2.62M distinct values in a ~303 MB symbol heap. So the
work is millions of random dereferences into a heap far larger than cache, each
followed by a short branchy substring scan — stalled on memory latency rather
than saturating bandwidth, which is precisely the shape a sibling thread fills.

The SMT advantage appears only where the heap stops fitting in cache:

distinct	-c 16	-c 8	SMT gain
1,000	6.8 ms	7.4 ms	+8.8%
100,000	8.6 ms	8.4 ms	−2% (none)
2,620,031	40.8 ms	44.8 ms	+9.8%

(The 1,000 row is 0.6 ms on a 7 ms query and does not fit the pattern; I would
not read anything into it.)

So q20 is the mirror image of the group-by scans: they stream fixed-width
columns at bandwidth and only contend on a sibling, while this one is latency-
bound and genuinely benefits. Both behaviours follow from the mechanism in your
platform.c comment.

Conclusion for the default: a carve-out keeping like over high-cardinality
SYM columns wide would rest on a real property. But it is one query, the
whole-suite effect is inside the noise on both my hosts, and I would still ship
the global physical-core default first and treat the carve-out as optional.

Separately, while measuring this I found that like applied to a vector
never dispatches to the worker pool at all, where the same predicate inside
select does — filed separately, since it is unrelated to the default decision.

[singaraiona]

Shipping it — #609. Two vendors, two microarchitectures, a third of the cycles
back for no measurable wall cost is a stronger case than I could build from one
box, and the three-machine agreement on q20 is a better answer to item 2 than I
asked for.

-c 8 is equivalent to the patch, to answer your question directly: the
change is one call, ray_thread_count() to ray_physical_core_count(), at the
single place the auto-sized pool picks its width. Your -c 8 produces the same
n_workers. Nothing else reads that value.

A correction I owe this issue

The +2.7% in the issue body above is wrong. The figure for this host is
+9.4%. I had compared a physical-cores run against a baseline captured hours
earlier while a CI runner was loading the machine — the baseline hot-sum was
5691 ms then and 2047 ms in the same session, and I did not notice the totals
moving by 2.8x. Re-measured with interleaved A/B runs to cancel drift:

rep 1   logical 2046.9 ms   physical 2243.4 ms   +9.6%
rep 2   logical 2046.6 ms   physical 2232.5 ms   +9.1%
rep 3   logical 2047.3 ms   physical 2241.4 ms   +9.5%

Baseline stable to 0.4 ms. So this box costs +9.4%, not +2.7% — and on the
strength of that wrong number I had provisionally decided not to ship. Your two
hosts are what reversed it. The correct reading is that 4 physical / 8 logical is
the unfavourable topology: it surrenders half its threads and has only four cores
to fall back on, and it is now documented in the commit as the case where an
operator should set -c instead.

What I confirmed on my side

Same mechanism, measured with counters rather than user CPU after your point
about thread-seconds — which applies to my numbers as much as yours:

	instructions	cycles	IPC	wall
logical (8 workers)	22.74 G	15.11 G	1.51	0.571 s
physical (4 workers)	22.71 G	7.50 G	3.20	0.559 s

Identical instructions, IPC halved, cache and LLC misses essentially unchanged
(LLC misses slightly down). Not memory pressure — the SMT signature.

That also disposes of my own explanation, and I should retract it explicitly
since you credited it: I said the cost was the main thread's completion spin
burning a core on stragglers. It is not. perf record at -c 8 puts 86.3% of
samples in hash_row_keys and the spin does not appear above 1.5%. My
supporting evidence for it — a U-shape where two tasks made both CPU and wall
worse — was the artifact you warned about: 1.58 s x 2 active threads = 3.16 ~
3.11 measured user CPU, so there was no spin overhead to see at all. Your metric
critique landed on my hypothesis, not just your own framing.

On your item-2 finding

The q20 analysis is the part I would not have got to. Cost tracking distinct
symbols rather than rows, the SMT advantage appearing only once the 303 MB heap
stops fitting in cache, and none at 100k distinct — that is a mechanism, not a
correlation, and it makes a like-over-high-cardinality-SYM carve-out a
principled thing rather than a fudge. I have written it into the commit as a
possible follow-up rather than bundling it: one query against a whole-suite
effect inside the noise does not justify topology-dependent branching in the
dispatch path yet. If a workload turns up where that scan dominates, the
measurement to justify it is already in your comment.

Also, your -c 8 advice for SMT hosts

Agreed and worth repeating for anyone reading: on an SMT host, -c <physical cores> gets all the available wall benefit at a fraction of the cycles, and it
works today on every released build. This change only makes it the default.

Closing on merge of #609.