Phase 29: Pool Hotbox v2 Stats Prune - NO-OP (infrastructure ready)
Target: g_pool_hotbox_v2_stats atomics (12 total) in Pool v2 Result: 0.00% impact (code path inactive by default, ENV-gated) Verdict: NO-OP - Maintain compile-out for future-proofing Audit Results: - Classification: 12/12 TELEMETRY (100% observational) - Counters: alloc_calls, alloc_fast, alloc_refill, alloc_refill_fail, alloc_fallback_v1, free_calls, free_fast, free_fallback_v1, page_of_fail_* (4 failure counters) - Verification: All stats/logging only, zero flow control usage - Phase 28 lesson applied: Traced all usages, confirmed no CORRECTNESS Key Finding: Pool v2 OFF by default - Requires HAKMEM_POOL_V2_ENABLED=1 to activate - Benchmark never executes Pool v2 code paths - Compile-out has zero performance impact (code never runs) Implementation (future-ready): - Added HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED (default: 0) - Wrapped 13 atomic write sites in core/hakmem_pool.c - Pattern: #if HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED ... #endif - Expected impact if Pool v2 enabled: +0.3~0.8% (HOT+WARM atomics) A/B Test Results: - Baseline (COMPILED=0): 52.98 M ops/s (±0.43M, 0.81% stdev) - Research (COMPILED=1): 53.31 M ops/s (±0.80M, 1.50% stdev) - Delta: -0.62% (noise, not real effect - code path not active) Critical Lesson Learned (NEW): Phase 29 revealed ENV-gated features can appear on hot paths but never execute. Updated audit checklist: 1. Classify atomics (CORRECTNESS vs TELEMETRY) 2. Verify no flow control usage 3. NEW: Verify code path is ACTIVE in benchmark (check ENV gates) 4. Implement compile-out 5. A/B test Verification methods added to documentation: - rg "getenv.*FEATURE" to check ENV gates - perf record/report to verify execution - Debug printf for quick validation Cumulative Progress (Phase 24-29): - Phase 24 (class stats): +0.93% GO - Phase 25 (free stats): +1.07% GO - Phase 26 (diagnostics): -0.33% NEUTRAL - Phase 27 (unified cache): +0.74% GO - Phase 28 (bg spill): NO-OP (all CORRECTNESS) - Phase 29 (pool v2): NO-OP (inactive code path) - Total: 17 atomics removed, +2.74% improvement Documentation: - PHASE29_POOL_HOTBOX_V2_AUDIT.md: Complete audit with TELEMETRY classification - PHASE29_POOL_HOTBOX_V2_STATS_RESULTS.md: Results + new lesson learned - ATOMIC_PRUNE_CUMULATIVE_SUMMARY.md: Updated with Phase 29 + new checklist - PHASE29_COMPLETE.md: Completion summary with recommendations Decision: Keep compile-out despite NO-OP - Code cleanliness (binary size reduction) - Future-proofing (ready when Pool v2 enabled) - Consistency with Phase 24-28 pattern Generated with Claude Code https://claude.com/claude-code Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Moe Charm (CI)
@ -147,6 +147,62 @@ if ((int)qlen < g_bg_spill_target) { // FLOW CONTROL DECISION
|
||||
|
||||
---
|
||||
|
||||
### Phase 29: Pool Hotbox v2 Stats Atomic Audit ✅ **NO-OP (Code Not Active)**
|
||||
|
||||
**Date:** 2025-12-16
|
||||
**Target:** Pool Hotbox v2 stats atomics (`g_pool_hotbox_v2_stats[ci].*`)
|
||||
**Files:** `core/hakmem_pool.c`, `core/box/pool_hotbox_v2_box.h`
|
||||
**Atomics:** 12 atomic counters (alloc_calls, free_calls, alloc_fast, free_fast, etc.)
|
||||
**Build Flag:** `HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED` (default: 0)
|
||||
|
||||
**Audit Results:**
|
||||
- **CORRECTNESS Atomics:** 0/12 (0%)
|
||||
- **TELEMETRY Atomics:** 12/12 (100%)
|
||||
- **Verdict:** **NO-OP** (code path not active)
|
||||
|
||||
**Analysis:**
|
||||
All 12 atomics are pure TELEMETRY (destructor dump only, no flow control). However, Pool Hotbox v2 is **disabled by default** via `HAKMEM_POOL_V2_ENABLED` environment variable, so these atomics are **never executed** in the benchmark.
|
||||
|
||||
**A/B Test Results (Anomaly Detected):**
|
||||
- **Baseline (compiled-out):** 52.98 M ops/s (±0.43M)
|
||||
- **Compiled-in:** 53.31 M ops/s (±0.80M)
|
||||
- **Improvement:** **-0.62%** (compiled-in is faster!)
|
||||
|
||||
**Root Cause:** Pool v2 is OFF by default (ENV-gated):
|
||||
```c
|
||||
const char* e = getenv("HAKMEM_POOL_V2_ENABLED");
|
||||
g = (e && *e && *e != '0') ? 1 : 0; // Default: OFF
|
||||
```
|
||||
|
||||
**Result:** Atomics are never incremented → compile-out has **zero runtime effect**.
|
||||
|
||||
**Why anomaly (-0.62% faster with atomics ON)?**
|
||||
1. High variance (research build: 1.50% stdev vs baseline: 0.81%)
|
||||
2. Compiler optimization artifact (code layout, instruction cache alignment)
|
||||
3. Sample size (10 runs) insufficient to distinguish signal from noise
|
||||
4. **Conclusion:** Noise, not real effect
|
||||
|
||||
**Decision:** NEUTRAL - Keep compile-out for:
|
||||
- Code cleanliness (reduces binary size)
|
||||
- Future-proofing (ready if Pool v2 is enabled)
|
||||
- Consistency with Phase 24-28 pattern
|
||||
|
||||
**Key Lesson:** Before A/B testing, verify code is ACTIVE:
|
||||
```bash
|
||||
rg "getenv.*FEATURE" && echo "⚠️ ENV-gated, may be OFF"
|
||||
```
|
||||
|
||||
**Updated Audit Checklist:**
|
||||
1. ✅ Classify atomics (CORRECTNESS vs TELEMETRY)
|
||||
2. ✅ Verify no flow control usage
|
||||
3. **NEW:** ✅ Verify code path is ACTIVE in benchmark ← **Phase 29 lesson**
|
||||
4. Implement compile-out
|
||||
5. A/B test
|
||||
|
||||
**Reference:** `docs/analysis/PHASE29_POOL_HOTBOX_V2_STATS_RESULTS.md`
|
||||
|
||||
---
|
||||
|
||||
## Cumulative Impact
|
||||
|
||||
| Phase | Atomics Removed | Frequency | Impact | Status |
|
||||
@ -156,9 +212,13 @@ if ((int)qlen < g_bg_spill_target) { // FLOW CONTROL DECISION
|
||||
| 26 | 5 (diagnostics) | Low (edge cases) | -0.33% | NEUTRAL ✅ |
|
||||
| 27 | 6 (unified cache) | Medium (refills) | **+0.74%** | GO ✅ |
|
||||
| **28** | **0 (bg spill)** | **N/A (all CORRECTNESS)** | **N/A** | **NO-OP ✅** |
|
||||
| **29** | **0 (pool v2)** | **N/A (code not active)** | **0.00%** | **NO-OP ✅** |
|
||||
| **Total** | **17 atomics** | **Mixed** | **+2.74%** | **✅** |
|
||||
|
||||
**Key Insight:** Atomic frequency matters more than count. High-frequency atomics (Phase 24+25) provide measurable benefit (+0.93%, +1.07%). Medium-frequency atomics (Phase 27, WARM path) provide substantial benefit (+0.74%). Low-frequency atomics (Phase 26) provide cleanliness but no performance gain. **Correctness atomics are untouchable** (Phase 28).
|
||||
**Key Insights:**
|
||||
1. **Frequency matters more than count:** High-frequency atomics (Phase 24+25) provide measurable benefit (+0.93%, +1.07%). Medium-frequency atomics (Phase 27, WARM path) provide substantial benefit (+0.74%). Low-frequency atomics (Phase 26) provide cleanliness but no performance gain.
|
||||
2. **Correctness atomics are untouchable:** Phase 28 showed that lock-free queues and flow control counters must not be touched.
|
||||
3. **ENV-gated code paths need verification:** Phase 29 showed that compile-out of inactive code has zero performance impact. Always verify code is active before A/B testing.
|
||||
|
||||
---
|
||||
|
||||
@ -193,19 +253,32 @@ if ((int)qlen < g_bg_spill_target) { // FLOW CONTROL DECISION
|
||||
- **Operational counters:** Used for control flow decisions, UNTOUCHABLE
|
||||
- Example: `g_bg_spill_len` looks like telemetry but controls queue depth limits
|
||||
|
||||
### 6. Verify Code is Active (NEW: Phase 29 Lesson)
|
||||
- **Phase 29:** Pool v2 stats were all TELEMETRY but ENV-gated (default OFF)
|
||||
- Compile-out had **zero impact** because code never ran
|
||||
- **Before A/B testing:**
|
||||
1. Check for `getenv()` gates → may be OFF by default
|
||||
2. Add temporary debug printf to verify code path is hit
|
||||
3. Or use `perf record` to check if functions are called
|
||||
- **Anomaly:** Compiled-in was 0.62% faster (noise due to compiler artifacts, not real effect)
|
||||
|
||||
---
|
||||
|
||||
## Next Phase Candidates (Phase 29+)
|
||||
## Next Phase Candidates (Phase 30+)
|
||||
|
||||
### High Priority: Warm Path Atomics
|
||||
### Completed Audits
|
||||
|
||||
1. ~~**Background Spill Queue** (Phase 28)~~ ✅ **COMPLETE (NO-OP)**
|
||||
- **Result:** All CORRECTNESS atomics, no compile-out candidates
|
||||
- **Reason:** Lock-free queue + flow control counter
|
||||
|
||||
### Medium Priority: Warm-ish Path Atomics
|
||||
2. ~~**Pool Hotbox v2 Stats** (Phase 29)~~ ✅ **COMPLETE (NO-OP)**
|
||||
- **Result:** All TELEMETRY atomics, but code path not active (ENV-gated)
|
||||
- **Reason:** `HAKMEM_POOL_V2_ENABLED` defaults to OFF
|
||||
|
||||
2. **Remote Target Queue** (Phase 29 candidate)
|
||||
### High Priority: Warm Path Atomics
|
||||
|
||||
3. **Remote Target Queue** (Phase 30 candidate)
|
||||
- **Targets:** `g_remote_target_len[class_idx]` atomics
|
||||
- **File:** `core/hakmem_tiny_remote_target.c`
|
||||
- **Atomics:** `atomic_fetch_add/sub` on queue length
|
||||
@ -216,28 +289,20 @@ if ((int)qlen < g_bg_spill_target) { // FLOW CONTROL DECISION
|
||||
|
||||
### Low Priority: Cold Path Atomics
|
||||
|
||||
3. **SuperSlab OS Stats** (Phase 29+)
|
||||
4. **SuperSlab OS Stats** (Phase 30+)
|
||||
- **Targets:** `g_ss_os_alloc_calls`, `g_ss_os_madvise_calls`, etc.
|
||||
- **Files:** `core/box/ss_os_acquire_box.h`, `core/box/madvise_guard_box.c`
|
||||
- **Frequency:** Cold (init/mmap/madvise)
|
||||
- **Expected Gain:** <0.1%
|
||||
- **Priority:** LOW (code cleanliness only)
|
||||
|
||||
4. **Shared Pool Diagnostics** (Phase 30+)
|
||||
5. **Shared Pool Diagnostics** (Phase 31+)
|
||||
- **Targets:** `rel_c7_*`, `dbg_c7_*` (release/acquire logs)
|
||||
- **Files:** `core/hakmem_shared_pool_acquire.c`, `core/hakmem_shared_pool_release.c`
|
||||
- **Frequency:** Cold (shared pool operations)
|
||||
- **Expected Gain:** <0.1%
|
||||
- **Priority:** LOW
|
||||
|
||||
5. **Pool Hotbox v2 Stats** (Phase 31+)
|
||||
- **Targets:** `g_pool_hotbox_v2_stats[ci].*` counters
|
||||
- **File:** `core/hakmem_pool.c`
|
||||
- **Atomics:** ~15 stats counters (alloc_calls, free_calls, etc.)
|
||||
- **Frequency:** Medium-High (pool operations)
|
||||
- **Expected Gain:** +0.2-0.5% (if high-frequency)
|
||||
- **Priority:** MEDIUM
|
||||
|
||||
---
|
||||
|
||||
## Pattern Template (For Future Phases)
|
||||
@ -336,6 +401,11 @@ All atomic compile gates in `core/hakmem_build_flags.h`:
|
||||
#ifndef HAKMEM_HDR_META_FAST_COMPILED
|
||||
# define HAKMEM_HDR_META_FAST_COMPILED 0
|
||||
#endif
|
||||
|
||||
// Phase 29: Pool Hotbox v2 Stats (NO-OP - code not active)
|
||||
#ifndef HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED
|
||||
# define HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED 0
|
||||
#endif
|
||||
```
|
||||
|
||||
**Default State:** All flags = 0 (compiled-out, production-ready)
|
||||
@ -345,12 +415,12 @@ All atomic compile gates in `core/hakmem_build_flags.h`:
|
||||
|
||||
## Conclusion
|
||||
|
||||
**Total Progress (Phase 24+25+26+27+28):**
|
||||
- **Performance Gain:** +2.74% (Phase 24: +0.93%, Phase 25: +1.07%, Phase 26: NEUTRAL, Phase 27: +0.74%, Phase 28: NO-OP)
|
||||
**Total Progress (Phase 24+25+26+27+28+29):**
|
||||
- **Performance Gain:** +2.74% (Phase 24: +0.93%, Phase 25: +1.07%, Phase 26: NEUTRAL, Phase 27: +0.74%, Phase 28: NO-OP, Phase 29: NO-OP)
|
||||
- **Atomics Removed:** 17 telemetry atomics from hot/warm paths
|
||||
- **Phases Completed:** 5 phases (4 with changes, 1 audit-only)
|
||||
- **Phases Completed:** 6 phases (4 with changes, 2 audit-only)
|
||||
- **Code Quality:** Cleaner hot/warm paths, closer to mimalloc's zero-overhead principle
|
||||
- **Next Target:** Phase 29 (remote target queue or pool hotbox v2 stats)
|
||||
- **Next Target:** Phase 30 (remote target queue or other ACTIVE code paths)
|
||||
|
||||
**Key Success Factors:**
|
||||
1. Systematic audit and classification (CORRECTNESS vs TELEMETRY)
|
||||
@ -365,13 +435,14 @@ All atomic compile gates in `core/hakmem_build_flags.h`:
|
||||
- Focus on high-frequency paths, audit carefully for CORRECTNESS vs TELEMETRY
|
||||
- Document all verdicts for reproducibility
|
||||
|
||||
**Lessons from Phase 28:**
|
||||
- Not all atomic counters are telemetry
|
||||
- Flow control counters (e.g., `g_bg_spill_len`) are CORRECTNESS
|
||||
**Lessons from Phase 28+29:**
|
||||
- Not all atomic counters are telemetry (Phase 28: flow control counters are CORRECTNESS)
|
||||
- Flow control counters (e.g., `g_bg_spill_len`) are UNTOUCHABLE
|
||||
- Always trace how counter is used before classifying
|
||||
- Verify code path is ACTIVE before A/B testing (Phase 29: ENV-gated code has zero impact)
|
||||
|
||||
---
|
||||
|
||||
**Last Updated:** 2025-12-16
|
||||
**Status:** Phase 24+25+26+27 Complete (+2.74%), Phase 28 Audit Complete (NO-OP)
|
||||
**Status:** Phase 24+25+26+27 Complete (+2.74%), Phase 28+29 Audit Complete (NO-OP x2)
|
||||
**Maintained By:** Claude Sonnet 4.5
|
||||
|
||||
299
docs/analysis/PHASE29_POOL_HOTBOX_V2_AUDIT.md
Normal file
299
docs/analysis/PHASE29_POOL_HOTBOX_V2_AUDIT.md
Normal file
@ -0,0 +1,299 @@
|
||||
# Phase 29: Pool Hotbox v2 Stats Audit
|
||||
|
||||
## Executive Summary
|
||||
|
||||
**Date:** 2025-12-16
|
||||
**Objective:** Audit all `g_pool_hotbox_v2_stats[ci].*` atomic operations to classify as CORRECTNESS vs TELEMETRY
|
||||
**Result:** ALL 12 ATOMIC FIELDS ARE PURE TELEMETRY
|
||||
**Recommendation:** Proceed with compile-out (Step 1-3)
|
||||
|
||||
## Critical Lesson from Phase 28
|
||||
|
||||
Phase 28 revealed that `g_bg_spill_len` appeared to be telemetry but was actually flow control. We learned:
|
||||
- Variables named `*_len`, `*_count`, `*_fail` can be CORRECTNESS
|
||||
- Must trace **all usage sites**, not just writes
|
||||
- Any `if/while` condition usage → CORRECTNESS
|
||||
- Only `fprintf/printf` usage → TELEMETRY
|
||||
|
||||
## Structure Definition
|
||||
|
||||
```c
|
||||
// core/box/pool_hotbox_v2_box.h:16-29
|
||||
typedef struct PoolHotBoxV2Stats {
|
||||
_Atomic uint64_t alloc_calls; // Line 17
|
||||
_Atomic uint64_t alloc_fast; // Line 18
|
||||
_Atomic uint64_t alloc_refill; // Line 19
|
||||
_Atomic uint64_t alloc_refill_fail; // Line 20
|
||||
_Atomic uint64_t alloc_fallback_v1; // Line 21
|
||||
_Atomic uint64_t free_calls; // Line 22
|
||||
_Atomic uint64_t free_fast; // Line 23
|
||||
_Atomic uint64_t free_fallback_v1; // Line 24
|
||||
_Atomic uint64_t page_of_fail_header_missing; // Line 25
|
||||
_Atomic uint64_t page_of_fail_out_of_range; // Line 26
|
||||
_Atomic uint64_t page_of_fail_misaligned; // Line 27
|
||||
_Atomic uint64_t page_of_fail_unknown; // Line 28
|
||||
} PoolHotBoxV2Stats;
|
||||
```
|
||||
|
||||
**Total:** 12 atomic uint64_t fields
|
||||
|
||||
## Complete Usage Analysis
|
||||
|
||||
### Write Operations (13 sites)
|
||||
|
||||
All writes are `atomic_fetch_add_explicit(..., 1, memory_order_relaxed)` incrementing counters:
|
||||
|
||||
1. **Line 903:** `alloc_calls` - write only, in `pool_hotbox_v2_record_alloc()`
|
||||
2. **Line 908:** `alloc_refill` - write only, in `pool_hotbox_v2_record_alloc_refill()`
|
||||
3. **Line 913:** `alloc_refill_fail` - write only, in `pool_hotbox_v2_record_alloc_refill_fail()`
|
||||
4. **Line 918:** `alloc_fallback_v1` - write only, in `pool_hotbox_v2_record_alloc_fallback()`
|
||||
5. **Line 923:** `free_calls` - write only, in `pool_hotbox_v2_record_free()`
|
||||
6. **Line 932:** `free_fallback_v1` - write only, in `pool_hotbox_v2_record_free_fallback()`
|
||||
7. **Line 947:** `page_of_fail_header_missing` - write only, in `pool_hotbox_v2_record_pageof_fail()`
|
||||
8. **Line 950:** `page_of_fail_out_of_range` - write only, in `pool_hotbox_v2_record_pageof_fail()`
|
||||
9. **Line 953:** `page_of_fail_misaligned` - write only, in `pool_hotbox_v2_record_pageof_fail()`
|
||||
10. **Line 957:** `page_of_fail_unknown` - write only, in `pool_hotbox_v2_record_pageof_fail()`
|
||||
11. **Line 1088:** `alloc_fast` - write only, in `pool_hotbox_v2_record_alloc_fast()`
|
||||
12. **Line 1093:** `free_fast` - write only, in `pool_hotbox_v2_record_free_fast()`
|
||||
|
||||
### Read Operations (12 sites)
|
||||
|
||||
All reads are `atomic_load_explicit(..., memory_order_relaxed)` in `pool_hotbox_v2_dump_stats()`:
|
||||
|
||||
13. **Line 1295:** `alloc_calls` - load for fprintf
|
||||
14. **Line 1296:** `alloc_refill` - load for fprintf
|
||||
15. **Line 1297:** `alloc_refill_fail` - load for fprintf
|
||||
16. **Line 1298:** `alloc_fallback_v1` - load for fprintf
|
||||
17. **Line 1299:** `free_calls` - load for fprintf
|
||||
18. **Line 1300:** `free_fallback_v1` - load for fprintf
|
||||
19. **Line 1301:** `alloc_fast` - load for fprintf
|
||||
20. **Line 1302:** `free_fast` - load for fprintf
|
||||
21. **Line 1303:** `page_of_fail_header_missing` - load for fprintf
|
||||
22. **Line 1304:** `page_of_fail_out_of_range` - load for fprintf
|
||||
23. **Line 1305:** `page_of_fail_misaligned` - load for fprintf
|
||||
24. **Line 1306:** `page_of_fail_unknown` - load for fprintf
|
||||
|
||||
### Critical Analysis: Are ANY reads used for flow control?
|
||||
|
||||
**NO.** Examined all read sites (lines 1295-1306):
|
||||
|
||||
```c
|
||||
// core/hakmem_pool.c:1292-1315
|
||||
__attribute__((destructor)) static void pool_hotbox_v2_dump_stats(void) {
|
||||
if (!pool_hotbox_v2_stats_enabled()) return;
|
||||
for (int i = 0; i < POOL_NUM_CLASSES; i++) {
|
||||
uint64_t ac = atomic_load_explicit(&g_pool_hotbox_v2_stats[i].alloc_calls, memory_order_relaxed);
|
||||
uint64_t ar = atomic_load_explicit(&g_pool_hotbox_v2_stats[i].alloc_refill, memory_order_relaxed);
|
||||
// ... [10 more loads]
|
||||
|
||||
// ONLY usage: fprintf condition check (line 1307)
|
||||
if (ac || afb || fc || ffb || ar || arf || af || ff || pf_hdr || pf_range || pf_mis || pf_unknown) {
|
||||
fprintf(stderr, "[POOL_V2_STATS] cls=%d alloc_calls=%llu ...\n", ...);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Analysis:**
|
||||
- The `if` condition (line 1307) checks if ANY counter is non-zero
|
||||
- **Purpose:** Skip printing empty lines (telemetry optimization)
|
||||
- **NOT flow control:** Does not affect allocation/free logic
|
||||
- **Effect if removed:** Would print zeros for inactive classes (cosmetic only)
|
||||
|
||||
## Field-by-Field Classification
|
||||
|
||||
| # | Field Name | Write Sites | Read Sites | Flow Control? | Classification |
|
||||
|---|------------|-------------|------------|---------------|----------------|
|
||||
| 1 | `alloc_calls` | 903 | 1295 | NO | **TELEMETRY** |
|
||||
| 2 | `alloc_fast` | 1088 | 1301 | NO | **TELEMETRY** |
|
||||
| 3 | `alloc_refill` | 908 | 1296 | NO | **TELEMETRY** |
|
||||
| 4 | `alloc_refill_fail` | 913 | 1297 | NO | **TELEMETRY** |
|
||||
| 5 | `alloc_fallback_v1` | 918 | 1298 | NO | **TELEMETRY** |
|
||||
| 6 | `free_calls` | 923 | 1299 | NO | **TELEMETRY** |
|
||||
| 7 | `free_fast` | 1093 | 1302 | NO | **TELEMETRY** |
|
||||
| 8 | `free_fallback_v1` | 932 | 1300 | NO | **TELEMETRY** |
|
||||
| 9 | `page_of_fail_header_missing` | 947 | 1303 | NO | **TELEMETRY** |
|
||||
| 10 | `page_of_fail_out_of_range` | 950 | 1304 | NO | **TELEMETRY** |
|
||||
| 11 | `page_of_fail_misaligned` | 953 | 1305 | NO | **TELEMETRY** |
|
||||
| 12 | `page_of_fail_unknown` | 957 | 1306 | NO | **TELEMETRY** |
|
||||
|
||||
## Detailed Evidence: No Flow Control Usage
|
||||
|
||||
### Evidence 1: Write-only call sites
|
||||
|
||||
All 12 write sites are in `pool_hotbox_v2_record_*()` helper functions:
|
||||
|
||||
```c
|
||||
// Line 901-904: Example write pattern
|
||||
static inline void pool_hotbox_v2_record_alloc(uint32_t ci) {
|
||||
if ((int)ci >= POOL_NUM_CLASSES) return;
|
||||
atomic_fetch_add_explicit(&g_pool_hotbox_v2_stats[ci].alloc_calls, 1, memory_order_relaxed);
|
||||
}
|
||||
```
|
||||
|
||||
**No return value used.** No control flow affected.
|
||||
|
||||
### Evidence 2: Caller sites don't check results
|
||||
|
||||
Example callers:
|
||||
|
||||
```c
|
||||
// Line 1207: alloc path
|
||||
pool_hotbox_v2_record_alloc(class_idx); // void return, result ignored
|
||||
|
||||
// Line 1246: free path
|
||||
pool_hotbox_v2_record_free(class_idx); // void return, result ignored
|
||||
|
||||
// Line 1103: fast alloc
|
||||
pool_hotbox_v2_record_alloc_fast(ci); // void return, result ignored
|
||||
```
|
||||
|
||||
**No branching based on these calls.**
|
||||
|
||||
### Evidence 3: Only read site is destructor dump
|
||||
|
||||
```c
|
||||
// Line 1292: __attribute__((destructor))
|
||||
static void pool_hotbox_v2_dump_stats(void) {
|
||||
if (!pool_hotbox_v2_stats_enabled()) return;
|
||||
// ... only loads stats for fprintf
|
||||
}
|
||||
```
|
||||
|
||||
**Destructor runs at exit, cannot affect runtime flow control.**
|
||||
|
||||
### Evidence 4: Contrast with Phase 28 CORRECTNESS example
|
||||
|
||||
**Phase 28 `g_bg_spill_len` (CORRECTNESS):**
|
||||
```c
|
||||
// Used in if condition for flow control
|
||||
if (atomic_load(&g_bg_spill_len, ...) >= SPILL_THRESHOLD) {
|
||||
// SKIP draining - flow control!
|
||||
return;
|
||||
}
|
||||
```
|
||||
|
||||
**Phase 29 stats (TELEMETRY):**
|
||||
```c
|
||||
// Only used in destructor fprintf
|
||||
if (ac || fc || ...) { // cosmetic: skip empty lines
|
||||
fprintf(stderr, "[POOL_V2_STATS] ...\n", ac, fc, ...);
|
||||
}
|
||||
```
|
||||
|
||||
**Clear difference:** Phase 28 affected runtime behavior; Phase 29 only affects logging.
|
||||
|
||||
## Gray Zone Analysis
|
||||
|
||||
**Suspicious field names:**
|
||||
- `alloc_refill_fail` - Has "fail" in name (Phase 28 warning sign)
|
||||
- `page_of_fail_*` - Multiple "fail" counters
|
||||
|
||||
**Verification:**
|
||||
|
||||
1. **`alloc_refill_fail` (line 913):**
|
||||
```c
|
||||
// Line 1218-1220: Context
|
||||
void* base = cold.refill_page ? cold.refill_page(...) : NULL;
|
||||
if (!base || !bs || !cap) {
|
||||
pool_hotbox_v2_record_alloc_refill_fail(class_idx); // AFTER decision
|
||||
return NULL; // Already failed
|
||||
}
|
||||
```
|
||||
**Result:** Counter incremented AFTER failure detected. Not used for detection.
|
||||
|
||||
2. **`page_of_fail_*` (lines 947-957):**
|
||||
```c
|
||||
// Line 1250-1253: Context
|
||||
pool_page_v2* p = pool_hotbox_v2_page_of(ctx, class_idx, raw_block, &pageof_reason);
|
||||
if (!p) {
|
||||
pool_hotbox_v2_record_pageof_fail(class_idx, pageof_reason); // AFTER NULL check
|
||||
return 0; // Already decided to fail
|
||||
}
|
||||
```
|
||||
**Result:** Counters incremented AFTER failure, not used to decide failure.
|
||||
|
||||
**Conclusion:** Even "fail" counters are pure telemetry.
|
||||
|
||||
## Hot Path Analysis
|
||||
|
||||
**Where are these atomics hit?**
|
||||
|
||||
1. **`pool_hotbox_v2_alloc()` (line 1203):**
|
||||
- WARM path (per-allocation)
|
||||
- Increments: `alloc_calls`, `alloc_fast`, `alloc_refill`, `alloc_refill_fail`
|
||||
|
||||
2. **`pool_hotbox_v2_free()` (line 1244):**
|
||||
- WARM path (per-free)
|
||||
- Increments: `free_calls`, `free_fast`, `page_of_fail_*`
|
||||
|
||||
3. **`pool_hotbox_v2_alloc_fast()` (lines 1096, 1121):**
|
||||
- HOT path (TLS fast path)
|
||||
- Increments: `alloc_fast`
|
||||
|
||||
4. **`pool_hotbox_v2_record_free_fast()` (line 1091):**
|
||||
- HOT path (TLS fast path)
|
||||
- Increments: `free_fast`
|
||||
|
||||
**Impact:** HOT path atomics (alloc_fast, free_fast) have highest tax. WARM path atomics also significant.
|
||||
|
||||
## Comparison: Phase 27 vs Phase 29
|
||||
|
||||
| Metric | Phase 27 (Unified Cache) | Phase 29 (Pool Hotbox v2) |
|
||||
|--------|--------------------------|---------------------------|
|
||||
| Total atomic fields | ~8 | 12 |
|
||||
| TELEMETRY fields | 8 | 12 |
|
||||
| CORRECTNESS fields | 0 | 0 |
|
||||
| Path hotness | WARM | HOT + WARM |
|
||||
| A/B result | +0.74% GO | TBD |
|
||||
|
||||
**Expectation:** Phase 29 should have similar or higher impact due to HOT path presence.
|
||||
|
||||
## Final Verdict
|
||||
|
||||
**Classification:** ALL 12 fields are **TELEMETRY**
|
||||
|
||||
**Evidence strength:** DEFINITIVE
|
||||
- No flow control usage (searched entire file)
|
||||
- No return value checks
|
||||
- Only destructor reads for fprintf
|
||||
- All writes are fire-and-forget increments
|
||||
|
||||
**Recommendation:** Proceed to Step 1 (build flag), Step 2 (compile-out), Step 3 (A/B test)
|
||||
|
||||
**Expected impact:** +0.2% to +0.5% (conservative estimate based on Phase 27)
|
||||
|
||||
## Files Involved
|
||||
|
||||
- **Stats definition:** `core/box/pool_hotbox_v2_box.h:16-29`
|
||||
- **Stats usage:** `core/hakmem_pool.c:813, 903-957, 1088-1315`
|
||||
- **Compile flag location:** `core/hakmem_build_flags.h` (to be added)
|
||||
|
||||
## Appendix: Complete Usage Map
|
||||
|
||||
```
|
||||
Write sites (13):
|
||||
903: alloc_calls (pool_hotbox_v2_record_alloc)
|
||||
908: alloc_refill (pool_hotbox_v2_record_alloc_refill)
|
||||
913: alloc_refill_fail (pool_hotbox_v2_record_alloc_refill_fail)
|
||||
918: alloc_fallback_v1 (pool_hotbox_v2_record_alloc_fallback)
|
||||
923: free_calls (pool_hotbox_v2_record_free)
|
||||
932: free_fallback_v1 (pool_hotbox_v2_record_free_fallback)
|
||||
947: page_of_fail_header_missing (pool_hotbox_v2_record_pageof_fail)
|
||||
950: page_of_fail_out_of_range (pool_hotbox_v2_record_pageof_fail)
|
||||
953: page_of_fail_misaligned (pool_hotbox_v2_record_pageof_fail)
|
||||
957: page_of_fail_unknown (pool_hotbox_v2_record_pageof_fail)
|
||||
1088: alloc_fast (pool_hotbox_v2_record_alloc_fast)
|
||||
1093: free_fast (pool_hotbox_v2_record_free_fast)
|
||||
|
||||
Read sites (12):
|
||||
1295-1306: All 12 fields loaded in pool_hotbox_v2_dump_stats() destructor
|
||||
|
||||
Control flow sites: NONE
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
**Audit completed:** 2025-12-16
|
||||
**Auditor:** Claude Sonnet 4.5
|
||||
**Next step:** Implement compile-out (Step 1-3)
|
||||
238
docs/analysis/PHASE29_POOL_HOTBOX_V2_STATS_RESULTS.md
Normal file
238
docs/analysis/PHASE29_POOL_HOTBOX_V2_STATS_RESULTS.md
Normal file
@ -0,0 +1,238 @@
|
||||
# Phase 29: Pool Hotbox v2 Stats Prune - Results
|
||||
|
||||
## Executive Summary
|
||||
|
||||
**Date:** 2025-12-16
|
||||
**Result:** **NO-OP** (Pool Hotbox v2 not active in default configuration)
|
||||
**Verdict:** NEUTRAL - Keep compile-out for code cleanliness
|
||||
**Impact:** 0.00% (atomics never executed)
|
||||
|
||||
## A/B Test Results
|
||||
|
||||
### Configuration
|
||||
|
||||
- **Baseline (COMPILED=0, default):** Atomics compiled-out (via `#if HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED`)
|
||||
- **Research (COMPILED=1):** Atomics active (`atomic_fetch_add_explicit` executed)
|
||||
- **Workload:** `bench_random_mixed` (10 runs, 20M ops each)
|
||||
|
||||
### Raw Data
|
||||
|
||||
**Baseline (atomics OFF):**
|
||||
```
|
||||
Run 1: 52,651,025 ops/s
|
||||
Run 2: 52,251,016 ops/s
|
||||
Run 3: 52,545,864 ops/s
|
||||
Run 4: 53,765,007 ops/s
|
||||
Run 5: 53,284,121 ops/s
|
||||
Run 6: 52,982,021 ops/s
|
||||
Run 7: 53,073,218 ops/s
|
||||
Run 8: 52,844,359 ops/s
|
||||
Run 9: 53,238,262 ops/s
|
||||
Run 10: 53,150,487 ops/s
|
||||
|
||||
Mean: 52,978,538 ops/s
|
||||
Stdev: 429,428 ops/s (0.81%)
|
||||
```
|
||||
|
||||
**Research (atomics ON):**
|
||||
```
|
||||
Run 1: 53,282,648 ops/s
|
||||
Run 2: 53,973,577 ops/s
|
||||
Run 3: 52,681,322 ops/s
|
||||
Run 4: 54,175,703 ops/s
|
||||
Run 5: 52,841,032 ops/s
|
||||
Run 6: 53,461,187 ops/s
|
||||
Run 7: 52,268,525 ops/s
|
||||
Run 8: 53,799,964 ops/s
|
||||
Run 9: 52,147,517 ops/s
|
||||
Run 10: 54,432,544 ops/s
|
||||
|
||||
Mean: 53,306,402 ops/s
|
||||
Stdev: 800,321 ops/s (1.50%)
|
||||
```
|
||||
|
||||
### Statistical Analysis
|
||||
|
||||
| Metric | Baseline (OFF) | Research (ON) | Delta |
|
||||
|--------|----------------|---------------|-------|
|
||||
| Mean | 52,978,538 ops/s | 53,306,402 ops/s | -327,864 ops/s |
|
||||
| Stdev | 429,428 (0.81%) | 800,321 (1.50%) | +370,893 (noise) |
|
||||
| **Relative Delta** | - | - | **-0.62%** |
|
||||
|
||||
**Interpretation:** Research build (atomics ON) is 0.62% FASTER than baseline (atomics OFF).
|
||||
|
||||
## Root Cause Analysis: Why NO-OP?
|
||||
|
||||
### Discovery
|
||||
|
||||
Pool Hotbox v2 is **OFF by default** and gated by environment variable:
|
||||
|
||||
```c
|
||||
// core/hakmem_pool.c:824-831
|
||||
static int pool_hotbox_v2_global_enabled(void) {
|
||||
static int g = -1;
|
||||
if (__builtin_expect(g == -1, 0)) {
|
||||
const char* e = getenv("HAKMEM_POOL_V2_ENABLED"); // ← ENV gate
|
||||
g = (e && *e && *e != '0') ? 1 : 0;
|
||||
}
|
||||
return g;
|
||||
}
|
||||
```
|
||||
|
||||
**Result:** All `pool_hotbox_v2_record_*()` calls are no-ops:
|
||||
- `pool_hotbox_v2_alloc()` is never called
|
||||
- `pool_hotbox_v2_free()` is never called
|
||||
- All 12 atomic counters are never incremented
|
||||
- Compile-out has **zero runtime effect**
|
||||
|
||||
### Why Research Build is Faster
|
||||
|
||||
**Hypothesis:** Compiler optimization artifact (noise)
|
||||
|
||||
1. **High variance in research build:** 1.50% stdev vs 0.81% baseline
|
||||
- Suggests measurement noise, not real effect
|
||||
- Delta (-0.62%) is within 1 stdev of research build
|
||||
|
||||
2. **Code layout changes:**
|
||||
- Adding `#if` guards changes object file layout
|
||||
- May affect instruction cache alignment by chance
|
||||
- LTO/PGO sensitive to code structure
|
||||
|
||||
3. **Sample size:** 10 runs insufficient to distinguish noise from signal
|
||||
|
||||
**Conclusion:** The -0.62% "speedup" for atomics ON is likely **noise**, not a real effect.
|
||||
|
||||
## Comparison: Phase 27 vs Phase 29
|
||||
|
||||
| Phase | Target | Path | Active? | Result |
|
||||
|-------|--------|------|---------|--------|
|
||||
| 27 | Unified Cache stats | WARM | ✅ YES | +0.74% GO |
|
||||
| 29 | Pool Hotbox v2 stats | HOT+WARM | ❌ NO | 0.00% NO-OP |
|
||||
|
||||
**Key difference:** Phase 27 stats were on ACTIVE code path; Phase 29 stats are on INACTIVE (ENV-gated) path.
|
||||
|
||||
## Why Keep Compile-Out?
|
||||
|
||||
Despite NO-OP result, **we maintain the compile-out** for:
|
||||
|
||||
1. **Code cleanliness:** Reduces binary size (12 atomics × 7 classes = 84 atomic counters)
|
||||
2. **Future-proofing:** If Pool v2 is enabled later, compile-out is already in place
|
||||
3. **Consistency:** Matches Phase 24-28 atomic prune pattern
|
||||
4. **Documentation:** Makes it clear these are research-only counters
|
||||
|
||||
## Actionable Findings
|
||||
|
||||
### For Phase 29
|
||||
|
||||
**Decision:** NEUTRAL - Maintain compile-out (default `COMPILED=0`)
|
||||
|
||||
**Rationale:**
|
||||
- No performance impact (code not running)
|
||||
- No harm (compile-out is correct for inactive code)
|
||||
- Future benefit (ready if Pool v2 is enabled)
|
||||
|
||||
### For Future Phases
|
||||
|
||||
**Lesson:** Before A/B testing compile-out, verify code is ACTIVE:
|
||||
|
||||
```bash
|
||||
# Check if feature is runtime-enabled
|
||||
rg "getenv.*FEATURE" && echo "⚠️ ENV-gated, may be OFF by default"
|
||||
|
||||
# Verify code path is exercised
|
||||
# Option 1: Add temporary printf, check if it fires
|
||||
# Option 2: Use perf to check if functions are called
|
||||
```
|
||||
|
||||
**Updated audit checklist:**
|
||||
1. ✅ Classify atomics (CORRECTNESS vs TELEMETRY)
|
||||
2. ✅ Verify no flow control usage
|
||||
3. **NEW:** ✅ Verify code path is ACTIVE in benchmark
|
||||
4. Implement compile-out
|
||||
5. A/B test
|
||||
|
||||
## Files Modified
|
||||
|
||||
### Phase 29 Implementation
|
||||
|
||||
1. **Build flag:** `core/hakmem_build_flags.h:352-361`
|
||||
```c
|
||||
#ifndef HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED
|
||||
# define HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED 0
|
||||
#endif
|
||||
```
|
||||
|
||||
2. **Compile-out:** `core/hakmem_pool.c:903-1129`
|
||||
- Wrapped 13 atomic writes (lines 903, 913, 922, 931, 941, 947, 950, 953, 957, 972-983, 1117, 1126)
|
||||
- Example:
|
||||
```c
|
||||
#if HAKMEM_POOL_HOTBOX_V2_STATS_COMPILED
|
||||
atomic_fetch_add_explicit(&g_pool_hotbox_v2_stats[ci].alloc_calls, 1, ...);
|
||||
#else
|
||||
(void)0;
|
||||
#endif
|
||||
```
|
||||
|
||||
3. **Include:** `core/hakmem_pool.c:48`
|
||||
- Added `#include "hakmem_build_flags.h"`
|
||||
|
||||
### Audit Documentation
|
||||
|
||||
- `docs/analysis/PHASE29_POOL_HOTBOX_V2_AUDIT.md`
|
||||
- Complete usage analysis (24 sites: 12 writes + 12 reads)
|
||||
- TELEMETRY classification (all 12 fields)
|
||||
- No CORRECTNESS usage found
|
||||
|
||||
## Performance Impact
|
||||
|
||||
**Expected:** +0.2% to +0.5% (similar to Phase 27)
|
||||
**Actual:** 0.00% (code path not active)
|
||||
|
||||
**If Pool v2 were enabled:**
|
||||
- 12 atomic counters on HOT+WARM path
|
||||
- Estimated impact: +0.3% to +0.8% (higher than Phase 27 due to HOT path presence)
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Immediate
|
||||
|
||||
1. **Keep compile-out:** No downside, future upside
|
||||
2. **Update audit process:** Add "verify code is active" step
|
||||
3. **Document ENV gates:** Tag all ENV-gated features in audit
|
||||
|
||||
### Future Work
|
||||
|
||||
**Phase 30+ candidates:**
|
||||
- Focus on **ACTIVE** code paths only
|
||||
- Check for ENV gates before scheduling A/B tests
|
||||
- Consider enabling Pool v2 (if performance gain expected) to test this prune's true impact
|
||||
|
||||
### Pool Hotbox v2 Activation
|
||||
|
||||
**If enabling Pool v2 in future:**
|
||||
|
||||
```bash
|
||||
# Enable Pool v2 globally
|
||||
export HAKMEM_POOL_V2_ENABLED=1
|
||||
|
||||
# Enable specific classes (bitmask)
|
||||
export HAKMEM_POOL_V2_CLASSES=0x7F # All 7 classes
|
||||
|
||||
# Enable stats (if COMPILED=1)
|
||||
export HAKMEM_POOL_V2_STATS=1
|
||||
```
|
||||
|
||||
**Then re-run Phase 29 A/B test to measure true impact.**
|
||||
|
||||
## Conclusion
|
||||
|
||||
Phase 29 successfully implements compile-out infrastructure for Pool Hotbox v2 stats, but has **zero performance impact** because Pool v2 is disabled by default in the benchmark.
|
||||
|
||||
**Verdict:** NEUTRAL - Maintain compile-out for code cleanliness and future-proofing.
|
||||
|
||||
**Key lesson:** Always verify code path is ACTIVE before scheduling A/B tests. ENV-gated features may appear on hot paths but never execute.
|
||||
|
||||
---
|
||||
|
||||
**Phase 29 Status:** COMPLETE (NO-OP, but infrastructure ready)
|
||||
**Next Phase:** Phase 30 (TBD - focus on ACTIVE code paths)
|
||||
Reference in New Issue
Block a user