perf(pool): size the auto pool by physical cores, not logical CPUs

docs/docs/language/repl.md

@@ -17,7 +17,7 @@ The `rayforce` binary operates in three modes depending on how it is invoked:
 echo '(+ 1 2)' | ./rayforce
 ```
 
-Interactive mode is detected automatically when stdin is a terminal. On startup the REPL prints a banner with the version, CPU, memory, logical CPU count, and worker count, then shows the `‣` prompt. By default the pool includes every online logical CPU, including SMT threads. Use `-c N` to choose a total worker count explicitly. Individual operations may use fewer tasks when their workload or memory budget requires it.
+Interactive mode is detected automatically when stdin is a terminal. On startup the REPL prints a banner with the version, CPU, memory, logical CPU count, and worker count, then shows the `‣` prompt. By default the pool includes one worker per **physical** core, excluding SMT siblings: two SMT threads share one core's issue bandwidth, so the second runs the same work at roughly half the IPC rather than adding throughput. Use `-c N` to choose a total worker count explicitly — worth doing on a latency-bound service on a low core-count SMT host, where the wider pool can still win wall-clock time at a higher CPU cost. Individual operations may use fewer tasks when their workload or memory budget requires it.
 
 ## REPL Commands
 

docs/grouping-engine-scaling-plan.md

@@ -78,7 +78,8 @@ settings and three rounds; immutable binary hashes accompany the process records
    validate a dominant null key and a dominant non-null key.
 4. Keep strategy choice capability-driven. Add shared updates only where safe
    and measured; contended atomic sums are not an automatic replacement.
-5. Check default uses all logical CPUs; distinguish pool size from bounded tasks.
+5. Check default uses all physical cores (SMT siblings excluded, #606);
+   distinguish pool size from bounded tasks.
 
 Exit: correct unary, binary and mixed streaming results; repeated synthetic
 8/default scaling measurements for min, sum, count and statistics. Investigate

src/core/platform.c

@@ -757,7 +757,8 @@ void* ray_vm_alloc_aligned(size_t size, size_t alignment) {
 bool ray_vm_hugepage(void* ptr, size_t size) { (void)ptr; (void)size; return false; }
 
 /* Threading — return errors / 1.  pool.c with n_workers==0 (the result of
- * thread_count==1 ⇒ ncpu-1 == 0) never invokes thread_create. */
+ * the auto-size path reading 1 core ⇒ ncpu-1 == 0) never invokes
+ * thread_create. */
 ray_err_t ray_thread_create(ray_thread_t* t, ray_thread_fn fn, void* arg) {
     (void)t; (void)fn; (void)arg;
     return RAY_ERR_NYI;
@@ -769,6 +770,12 @@ ray_err_t ray_thread_join(ray_thread_t t) {
 }
 
 uint32_t ray_thread_count(void) { return 1; }
+
+/* WASM: single-threaded, so physical and logical are both 1.  Defined here
+ * because pool.c's auto-size path calls it on every platform (#606) and
+ * platform.h declares it unconditionally. */
+uint32_t ray_physical_core_count(void) { return ray_thread_count(); }
+
 uint64_t ray_cache_llc_bytes(void) { return 0; }
 
 /* Semaphore — counter-only.  Single-threaded so wait never blocks (the

src/core/pool.c

@@ -154,9 +154,29 @@ static ray_err_t ray_pool_create_impl(ray_pool_t* pool, uint32_t n_workers,
             long v = strtol(env, NULL, 10);
             n_workers = (v > 0) ? (uint32_t)v : 0;
         } else {
-            /* 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();
+            /* Default to every PHYSICAL core, not every logical CPU (#606).
+             * Two SMT siblings split one core's issue bandwidth, so the second
+             * thread re-runs the same instruction stream at roughly half the
+             * IPC rather than adding throughput.  Measured on the full
+             * ClickBench suite (10M rows, 43 queries x 3, three repetitions):
+             *
+             *   host                    cycles    instructions   hot times
+             *   Ryzen AI 7 350 (8/16)   -31.6%    -0.4%          -1.6%
+             *   i7-10700K      (8/16)   -34.4%    -0.3%          -0.8%
+             *
+             * Instructions flat is the point: the saving is stall cycles, not
+             * work removed, and the wall-time column is inside the run-to-run
+             * spread on both hosts.  A 4-physical/8-logical i7-6700 is the
+             * unfavourable case — it surrenders half its threads and pays
+             * ~9% on hot times for the same ~50% cycle saving — so low
+             * core-count SMT hosts that are latency-bound should set -c
+             * explicitly.
+             *
+             * ray_physical_core_count falls back to the logical count when the
+             * topology cannot be read, so exotic systems keep the old
+             * behaviour.  Individual operations may still bound their task
+             * count further to their workload or memory budget. */
+            uint32_t ncpu = ray_physical_core_count();
             n_workers = (ncpu > 1) ? ncpu - 1 : 0;
         }
     }

test/test_pool.c

@@ -1379,7 +1379,15 @@ static test_result_t test_epoll_hup_no_errfn(void) {
  * -------------------------------------------------------------------------- */
 
 #if defined(__linux__) || defined(__APPLE__)
-static test_result_t test_auto_all_logical_cpus(void) {
+/* The auto-sized pool (RAYFORCE_CORES unset, ray_pool_create(.., 0)) recruits
+ * one participant per PHYSICAL core, not per logical CPU (#606).  Two SMT
+ * siblings share one core's issue bandwidth, so the second thread re-runs the
+ * same instruction stream at roughly half the IPC instead of adding
+ * throughput: measured across three microarchitectures, about a third of the
+ * cycles come back for no measurable wall-time cost.
+ * ray_physical_core_count falls back to the logical count when the topology
+ * cannot be read, so this assertion holds either way. */
+static test_result_t test_auto_physical_cores(void) {
     const char* current = getenv("RAYFORCE_CORES");
     char* saved = current ? strdup(current) : NULL;
     TEST_ASSERT_TRUE(!current || saved);
@@ -1390,14 +1398,21 @@ static test_result_t test_auto_all_logical_cpus(void) {
     if (rc == RAY_OK) ray_pool_free(&local);
     if (saved) { setenv("RAYFORCE_CORES", saved, 1); free(saved); }
     TEST_ASSERT_EQ_I(rc, RAY_OK);
-    TEST_ASSERT_EQ_I(total, ray_thread_count());
+    TEST_ASSERT_EQ_I(total, ray_physical_core_count());
+    /* The assertion above shares a function with the code under test, so on a
+     * non-SMT runner (physical == logical, the common CI case) it cannot tell
+     * the new policy from the old one.  Pinning the relationship as well means
+     * that on an SMT host — where the two differ — a revert to the logical
+     * count fails here rather than passing quietly. */
+    TEST_ASSERT_TRUE(ray_physical_core_count() <= ray_thread_count());
+    TEST_ASSERT_TRUE(total <= ray_thread_count());
     PASS();
 }
 #endif
 
 const test_entry_t pool_entries[] = {
 #if defined(__linux__) || defined(__APPLE__)
-    { "pool/auto_all_logical_cpus", test_auto_all_logical_cpus, NULL, NULL },
+    { "pool/auto_physical_cores",   test_auto_physical_cores,   NULL, NULL },
 #endif
     { "pool/parallel_sum", test_parallel_sum, NULL, NULL },
     { "pool/parallel_add", test_parallel_add, NULL, NULL },