test(thread-pool): document and guard constructor behavior

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: luoxiaojian

THREAD_POOL_BEHAVIOR.md

@@ -0,0 +1,313 @@
+# ThreadPool Constructor and Configuration Behavior
+
+## Why this changed
+
+`ThreadPool` previously exposed both of these constructors:
+
+```cpp
+ThreadPool(uint32_t size, bool binding);
+ThreadPool(bool binding);
+```
+
+Calls such as `ThreadPool(1)` selected the exact `bool` overload instead of
+converting `1` to `uint32_t`. As a result, a caller intending to create one
+thread actually created `hardware_concurrency()` threads with CPU binding
+enabled. Any positive one-argument thread count had the same problem.
+
+The ambiguous overload was removed. Thread count and CPU binding must now be
+specified together:
+
+```cpp
+ThreadPool(uint32_t size, bool binding);
+```
+
+## Direct C++ API
+
+| Construction | Thread count | CPU binding |
+|---|---:|---|
+| `ThreadPool()` | `max(std::thread::hardware_concurrency(), 1)` | Disabled |
+| `ThreadPool(n, false)` | `n` | Disabled |
+| `ThreadPool(n, true)` | `n` | Enabled |
+
+The default constructor has not changed: it uses hardware concurrency, never
+zero, with binding disabled.
+
+The following one-argument forms are no longer supported:
+
+```cpp
+ThreadPool(n);
+ThreadPool(true);
+ThreadPool(false);
+```
+
+Use explicit replacements:
+
+```cpp
+// Old: ThreadPool(false)
+ThreadPool();
+
+// Old: ThreadPool(true)
+ThreadPool(std::max(std::thread::hardware_concurrency(), 1u), true);
+
+// Intended meaning of old ThreadPool(n)
+ThreadPool(n, false);  // or true when binding is required
+```
+
+This is a C++ source compatibility change. It intentionally turns ambiguous
+calls into compile errors instead of silently choosing the wrong behavior.
+
+CPU binding is implemented on Linux excluding Android. On unsupported
+platforms, the binding flag does not change thread affinity.
+
+## DB global query and optimize pools
+
+The DB pools do not use the direct `ThreadPool()` default. Their counts are
+initialized independently from `CgroupUtil::getCpuLimit()`:
+
+| Configuration | Default count | Default binding |
+|---|---:|---|
+| Query pool | Cgroup CPU limit | Disabled |
+| Optimize pool | Cgroup CPU limit | Disabled |
+
+Therefore:
+
+- Omitting a DB thread count does **not** mean one thread.
+- On an unrestricted host, the cgroup CPU limit will commonly equal available
+  CPU capacity.
+- In a CPU-limited container, the DB defaults may differ from
+  `std::thread::hardware_concurrency()`.
+- Query and optimize counts are independent. Setting `query_threads` does not
+  implicitly change `optimize_threads`.
+
+Before this fix, the DB passed its configured count as a single constructor
+argument. Every valid positive count was converted to `true`, so the actual
+runtime behavior was hardware-concurrency threads with binding enabled. This
+meant configured values such as `optimize_threads=1` were ignored.
+
+After this fix, the configured count and binding are both forwarded
+explicitly, so the effective behavior matches the configuration.
+
+## Python API
+
+`zvec.init()` exposes the four independent options:
+
+```python
+zvec.init(
+    query_threads=None,
+    query_thread_binding=None,
+    optimize_threads=None,
+    optimize_thread_binding=None,
+)
+```
+
+Behavior:
+
+| Input | Effective behavior |
+|---|---|
+| Count is `None` | Use the corresponding cgroup-derived DB default |
+| Count is a positive integer | Use exactly that many threads |
+| Binding is `None` | Use the DB default (`False`) |
+| Binding is `False` | Do not bind worker threads |
+| Binding is `True` | Bind worker threads where supported |
+
+Examples:
+
+```python
+# Both pools use their cgroup-derived counts without binding.
+zvec.init()
+
+# Optimize uses exactly one unbound worker.
+zvec.init(optimize_threads=1)
+
+# Query uses four bound workers; optimize keeps its independent defaults.
+zvec.init(query_threads=4, query_thread_binding=True)
+```
+
+## C API
+
+The C configuration API now includes independent setters and getters:
+
+```c
+zvec_config_data_set_query_thread_binding(config, binding);
+zvec_config_data_get_query_thread_binding(config);
+zvec_config_data_set_optimize_thread_binding(config, binding);
+zvec_config_data_get_optimize_thread_binding(config);
+```
+
+If these setters are not called, binding remains disabled.
+
+## Internal call-site migration
+
+Internal call sites were migrated according to their previous intended
+behavior:
+
+- Benchmark tools with no requested count still use hardware concurrency
+  without binding.
+- Recall tools with no requested count still use hardware concurrency with
+  binding.
+- Explicit benchmark and recall thread counts are now honored.
+- `Index::Merge` uses `write_concurrency` without binding.
+- DB query and optimize pools use their configured count and binding.
+- Existing call sites already passing both count and binding are unchanged.
+
+The recall resize path previously used the arguments in reverse order:
+
+```cpp
+ThreadPool(true, threads);
+```
+
+That constructed one bound worker. It now correctly uses:
+
+```cpp
+ThreadPool(threads, true);
+```
+
+## Performance validation
+
+The fix was validated with an end-to-end Python DB API benchmark using the
+SIFT dataset:
+
+| Parameter | Value |
+|---|---|
+| Dataset | SIFT, 1,000,000 FP32 vectors, 128 dimensions |
+| Index | Vamana |
+| Search list size | 500 |
+| Alpha | 1.5 |
+| Quantizer | Uniform INT8 |
+| Optimize threads | 1 per process |
+| Build flow | `create_and_open -> insert -> optimize` |
+
+Two codebases were compared on the same machine with separately built wheels
+and virtual environments:
+
+- Before the fix: commit `a90ec5b1d658d`, wheel `0.5.2.dev30`.
+- After the fix: the ThreadPool fix and migrated call sites, wheel
+  `0.5.2.dev32`.
+
+### Observed runtime behavior
+
+The test machine exposes 64 logical CPUs. The benchmark called:
+
+```python
+zvec.init(optimize_threads=1)
+```
+
+It did not override the query thread count or either binding option.
+
+| Pool / behavior | Before the fix | After the fix |
+|---|---|---|
+| Query pool | 64 workers, bound to CPUs 0-63 | 64 workers, unbound |
+| Optimize pool | 64 workers, bound to CPUs 0-63 | 1 worker, unbound |
+| Total pool workers | 128 | 65 |
+| Observed process threads | Approximately 214 | Approximately 150 |
+| Workers doing Vamana build work | Effectively about 1 per process | 1 per process |
+| CPU used by each builder in the four-process phase | Approximately 25% of one CPU before competing builders completed | Approximately 92-100% of one CPU |
+
+The total process thread count is higher than the two global pools because it
+also includes Python, storage-engine, I/O, and other background threads. Those
+additional threads were present in both versions.
+
+Before the fix, both positive configuration values were passed through the
+one-argument constructor:
+
+```cpp
+ThreadPool(configured_count);
+```
+
+Both `64` and `1` converted to the Boolean value `true`. Consequently, both
+pools created 64 workers and enabled binding. `optimize_threads=1` therefore
+did not create one optimize worker.
+
+The Vamana build remained effectively single-threaded in this test: despite
+the 64 optimize workers, only about one worker per process was doing sustained
+CPU work. The other optimize workers and the entire query pool were mostly
+idle. This explains why the single-process result did not become faster in the
+old version.
+
+The excessive idle workers add thread stacks, scheduler bookkeeping, and some
+context-switch overhead, but they are not sufficient to explain the observed
+four-times slowdown. The dominant problem was CPU affinity:
+
+1. Every process created the same 64-worker optimize pool.
+2. Worker `i` in every process was bound to CPU `i`.
+3. The effectively active worker selected by each identical pool tended to
+   have the same worker index.
+4. Active build work from multiple processes therefore competed on the same
+   CPU instead of being spread across the 64 available CPUs.
+
+This behavior matches the timing curve. The degree-24 build, which overlapped
+with all four processes for almost its entire lifetime, took 4.28 times its
+single-process time before the fix. As shorter jobs completed, the remaining
+processes received a larger share of the contended CPU, so the longest
+degree-64 job showed a smaller but still substantial 2.45-times slowdown.
+
+After the fix, each process created one unbound optimize worker. The operating
+system could schedule the four runnable workers on different CPUs, and live
+sampling showed each builder consuming close to one full CPU. The query pool
+still contributed 64 mostly idle workers because the benchmark did not set
+`query_threads`; this accounts for much of the approximately 150-thread process
+total, but did not prevent scaling because those threads were unbound and idle.
+
+### Single-process results
+
+| Degree | Before | After | Change |
+|---:|---:|---:|---:|
+| 24 | 790.2 s | 777.8 s | -1.6% |
+| 64 | 2865.3 s | 2847.0 s | -0.6% |
+
+Single-process performance was effectively unchanged. This indicates that the
+fix does not alter Vamana's underlying build cost or introduce a measurable
+single-process regression.
+
+### Four-process results
+
+Four independent processes simultaneously built indexes with degrees 24, 32,
+48, and 64. Every process requested `optimize_threads=1`.
+
+| Degree | Before | After | Speedup | Time reduction |
+|---:|---:|---:|---:|---:|
+| 24 | 3378.6 s | 841.6 s | 4.01x | 75.1% |
+| 32 | 4282.3 s | 1137.5 s | 3.76x | 73.4% |
+| 48 | 5955.2 s | 1946.4 s | 3.06x | 67.3% |
+| 64 | 7009.0 s | 2975.1 s | 2.36x | 57.6% |
+
+The four-process group wall-clock time was determined by the degree-64 build:
+
+| Metric | Before | After | Improvement |
+|---|---:|---:|---:|
+| Group wall time | 7009.2 s (116m 49s) | 2975.4 s (49m 35s) | 57.5% lower |
+| Equivalent speedup | — | — | 2.36x |
+
+### Parallel overhead relative to a single process
+
+| Degree | Before fix | After fix |
+|---:|---:|---:|
+| 24 | +327.6% | +8.2% |
+| 64 | +144.6% | +4.5% |
+
+Before the fix, the degree-24 build took 4.28 times as long when sharing the
+machine with three other builders. After the fix it took only 1.08 times as
+long. The degree-64 ratio similarly improved from 2.45 times to 1.05 times.
+
+All indexes produced before and after the fix reached an index completeness of
+1.0. The results confirm that the change specifically removes multi-process
+thread contention while preserving single-process performance and index
+correctness.
+
+## Compatibility summary
+
+Behavior is unchanged for:
+
+- `ThreadPool()`;
+- `ThreadPool(n, false)`;
+- `ThreadPool(n, true)`;
+- internal call sites whose old behavior was already explicit and correct.
+
+Behavior intentionally changes for:
+
+- one-argument thread counts that were previously interpreted as booleans;
+- DB thread-count configuration that was previously ignored;
+- `Index::Merge` and recall resize paths affected by ambiguous or reversed
+  arguments;
+- DB binding defaults, which now explicitly default to disabled instead of
+  being accidentally enabled by overload resolution.

python/zvec/zvec.py

@@ -86,7 +86,8 @@ def init(
             Zvec's internal setting, currently ``False``.
         optimize_threads (Optional[int], optional):
             Threads for background tasks (e.g., compaction, indexing).
-            If ``None``, defaults to same as ``query_threads`` or CPU count.
+            If ``None`` (default), inferred independently from available CPU
+            cores (via cgroup).
         optimize_thread_binding (Optional[bool], optional):
             Whether to bind optimize worker threads to CPU cores. Defaults to
             Zvec's internal setting, currently ``False``.

tests/ailego/parallel/thread_pool_test.cc

@@ -15,11 +15,33 @@
 #include <chrono>
 #include <iostream>
 #include <memory>
+#include <type_traits>
 #include <gtest/gtest.h>
 #include <zvec/ailego/parallel/thread_pool.h>
 
 using namespace zvec::ailego;
 
+static_assert(!std::is_constructible<ThreadPool, uint32_t>::value,
+              "ThreadPool thread count must not be ambiguous with binding");
+static_assert(!std::is_constructible<ThreadPool, bool>::value,
+              "ThreadPool binding must require an explicit thread count");
+static_assert(std::is_constructible<ThreadPool, uint32_t, bool>::value,
+              "ThreadPool must accept an explicit count and binding");
+
+TEST(ThreadPool, ConstructorBehavior) {
+  const auto hardware_concurrency =
+      std::max(std::thread::hardware_concurrency(), 1u);
+
+  ThreadPool default_pool;
+  EXPECT_EQ(hardware_concurrency, default_pool.count());
+
+  ThreadPool single_unbound_pool(1, false);
+  EXPECT_EQ(1u, single_unbound_pool.count());
+
+  ThreadPool double_bound_pool(2, true);
+  EXPECT_EQ(2u, double_bound_pool.count());
+}
+
 struct A {
   A(void) : pool(std::make_shared<ThreadPool>()) {}
 

tests/db/common/config_test.cc

@@ -29,6 +29,15 @@ class ConfigTest : public ::testing::Test {
   }
 };
 
+TEST_F(ConfigTest, ThreadConfigDataDefaults) {
+  GlobalConfig::ConfigData config;
+
+  ASSERT_GT(config.query_thread_count, 0u);
+  ASSERT_EQ(config.query_thread_count, config.optimize_thread_count);
+  ASSERT_FALSE(config.query_thread_binding);
+  ASSERT_FALSE(config.optimize_thread_binding);
+}
+
 TEST_F(ConfigTest, InitializeWithDefaultConfig) {
   GlobalConfig::ConfigData config;