diff --git a/docs/docs/language/repl.md b/docs/docs/language/repl.md index 3b66f8c4..d048bd3d 100644 --- a/docs/docs/language/repl.md +++ b/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 diff --git a/docs/grouping-engine-scaling-plan.md b/docs/grouping-engine-scaling-plan.md index 9e83ac57..558318fd 100644 --- a/docs/grouping-engine-scaling-plan.md +++ b/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 diff --git a/src/core/platform.c b/src/core/platform.c index 89852918..e14deaa4 100644 --- a/src/core/platform.c +++ b/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 diff --git a/src/core/pool.c b/src/core/pool.c index c3cac800..98bef35f 100644 --- a/src/core/pool.c +++ b/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; } } diff --git a/test/test_pool.c b/test/test_pool.c index 8eb499a1..a9bcf889 100644 --- a/test/test_pool.c +++ b/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 },