Files
hakmem/docs/analysis/PHASE29_POOL_HOTBOX_V2_AUDIT.md

300 lines
11 KiB
Markdown
Raw Normal View History

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>
2025-12-16 06:33:41 +09:00
# 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)