path: root/doc/BUILD_OPTIMIZATION_PROPOSAL_V2.md

# Build Optimization Proposal V2: Core Library Iteration Speed

## Executive Summary (Revised Focus)

**Real Problem**: Changing common headers during development causes **4-5 second rebuilds** of 35+ files. Asset changes are infrequent and acceptable.

**Root Cause**: `asset_manager.h` is over-included. 17 files include it, but only 7 need the struct definitions (TextureAsset, MeshAsset).

**Solution**: Split `asset_manager.h` into core + extensions → reduce rebuild cascade from **35 files to ~7 files**.

---

## Revised Performance Baseline

### Developer Workflow Measurements
```
Single .cc file change:           0.65s ✅ (excellent)
Touch asset_manager.h:            4.82s ❌ (35 files rebuild)
Touch mini_math.h:                1.90s ⚠️  (23 files rebuild)
Touch object.h:                   ??? (needs measurement)
Asset changes:                    3.50s ✅ (acceptable - rare)
```

**Key Insight**: The bottleneck is **header dependencies**, not asset generation.

---

## Problem Analysis: asset_manager.h

### Current Structure
```cpp
// asset_manager.h (MONOLITHIC)
#pragma once
#include <cstddef>
#include <cstdint>

enum class AssetId : uint16_t;         // ✅ Already forward-declared

typedef bool (*ProcGenFunc)(...);

struct AssetRecord { ... };            // ❌ Rarely needed

const uint8_t* GetAsset(...);          // ✅ Frequently needed
void DropAsset(...);                    // ✅ Frequently needed

struct TextureAsset { ... };           // ❌ Only 2 files need this
struct MeshVertex { ... };             // ❌ Only 3 files need this
struct MeshAsset { ... };              // ❌ Only 3 files need this

TextureAsset GetTextureAsset(...);     // ❌ Only 2 files need this
MeshAsset GetMeshAsset(...);           // ❌ Only 3 files need this
```

### Who Includes It? (17 files)
```
✅ NEED BASIC (GetAsset only):
  - src/3d/renderer_skybox.cc
  - src/3d/renderer_sdf.cc
  - src/audio/audio.cc
  - src/audio/tracker.cc
  - src/gpu/effects/shaders.cc
  - src/audio/spectrogram_resource_manager.h
  - ... (10 files total)

❌ NEED STRUCTS (GetTextureAsset/GetMeshAsset):
  - src/3d/renderer_mesh.cc (MeshAsset)
  - src/3d/visual_debug.cc (MeshAsset)
  - src/gpu/effects/flash_cube_effect.cc (TextureAsset)
  - src/gpu/effects/hybrid_3d_effect.cc (TextureAsset)
  - ... (7 files total)
```

### Transitive Includes (The Real Problem!)
```
object.h
  #include "asset_manager.h"  ❌ Only needs AssetId (already forward-declared!)
    ↓
renderer.cc, renderer_sdf.cc, renderer_mesh.cc, renderer_skybox.cc
    ↓
ALL of 3d library rebuilds when asset_manager.h changes!
```

**Root Cause**: `object.h` unnecessarily includes full `asset_manager.h` for `AssetId mesh_asset_id`, but `AssetId` is already forward-declared!

---

## Proposed Solution: Split asset_manager.h

### New Structure

#### File 1: `asset_manager_fwd.h` (Forward Declarations)
```cpp
// asset_manager_fwd.h - LIGHTWEIGHT, SAFE TO INCLUDE EVERYWHERE
#pragma once
#include <cstddef>
#include <cstdint>

enum class AssetId : uint16_t;  // Just the forward declaration

typedef bool (*ProcGenFunc)(uint8_t*, int, int, const float*, int);

struct AssetRecord;  // Forward declaration (opaque)
```

#### File 2: `asset_manager.h` (Core API)
```cpp
// asset_manager.h - BASIC ASSET RETRIEVAL (most common use case)
#pragma once
#include "asset_manager_fwd.h"

struct AssetRecord {
  const uint8_t* data;
  size_t size;
  bool is_procedural;
  const char* proc_func_name_str;
  const float* proc_params;
  int num_proc_params;
};

// Core API - most files only need this
const uint8_t* GetAsset(AssetId asset_id, size_t* out_size = nullptr);
void DropAsset(AssetId asset_id, const uint8_t* asset);
```

#### File 3: `asset_manager_helpers.h` (Typed Asset Helpers)
```cpp
// asset_manager_helpers.h - SPECIALIZED HELPERS (only 7 files need this)
#pragma once
#include "asset_manager.h"

struct TextureAsset {
  int width;
  int height;
  const uint8_t* pixels;
};

struct MeshVertex {
  float p[3];
  float n[3];
  float u[2];
};

struct MeshAsset {
  uint32_t num_vertices;
  const MeshVertex* vertices;
  uint32_t num_indices;
  const uint32_t* indices;
};

TextureAsset GetTextureAsset(AssetId asset_id);
MeshAsset GetMeshAsset(AssetId asset_id);
```

### Migration Plan

#### Step 1: Fix object.h (QUICK WIN - 0 files)
```cpp
// object.h (BEFORE)
#include "util/asset_manager.h"  // ❌ Pulls in everything

// object.h (AFTER)
#include "util/asset_manager_fwd.h"  // ✅ Only AssetId forward declaration
```

**Impact**: Changing `asset_manager.h` no longer rebuilds entire 3d library!

#### Step 2: Update includes in all source files
```cpp
// Most files (10 files) - only need basic GetAsset
#include "util/asset_manager.h"

// Specialized files (7 files) - need struct helpers
#include "util/asset_manager_helpers.h"
```

**Automated Migration**:
```bash
# Files using GetTextureAsset or GetMeshAsset
for file in renderer_mesh.cc visual_debug.cc flash_cube_effect.cc hybrid_3d_effect.cc; do
  sed -i '' 's|asset_manager\.h|asset_manager_helpers.h|g' "src/**/$file"
done

# All other files - keep as-is (will include base asset_manager.h)
```

---

## Expected Performance Improvements

### Before Split
```
Touch asset_manager.h → 35 files rebuild → 4.82s
```

### After Split (Estimated)
```
Touch asset_manager_fwd.h  → 0 files rebuild (forward decl only) → 0.2s ✅
Touch asset_manager.h      → 10 files rebuild → 1.5s ✅
Touch asset_manager_helpers.h → 7 files rebuild → 1.0s ✅
```

**Worst Case**: 1.5s (vs 4.82s) = **69% faster** ⚡

**Typical Case**: Most changes to asset_manager internals won't affect the API → 0.2s

---

## Implementation Checklist

### Phase 1: Create New Headers (30 minutes)
- [ ] Create `src/util/asset_manager_fwd.h` (forward declarations only)
- [ ] Split `src/util/asset_manager.h` → keep core API
- [ ] Create `src/util/asset_manager_helpers.h` → move TextureAsset/MeshAsset

### Phase 2: Update Includes (30 minutes)
- [ ] Update `src/3d/object.h` → use `asset_manager_fwd.h`
- [ ] Update 7 files needing structs → use `asset_manager_helpers.h`:
  - `src/3d/renderer_mesh.cc`
  - `src/3d/visual_debug.cc`
  - `src/3d/visual_debug.h`
  - `src/gpu/effects/flash_cube_effect.cc`
  - `src/gpu/effects/hybrid_3d_effect.cc`
  - `src/tests/test_assets.cc`
  - `src/tests/test_mesh.cc`
- [ ] Update `src/util/asset_manager.cc` → include `asset_manager_helpers.h`

### Phase 3: Verify (15 minutes)
- [ ] Run all tests: `ctest`
- [ ] Measure rebuild time: `touch src/util/asset_manager.h && cmake --build build`
- [ ] Confirm <2s rebuild time

**Total Effort**: ~75 minutes

---

## Additional Low-Hanging Fruit

### 1. Check other transitive includes
**Action**: Audit `object.h` for other unnecessary includes
```bash
# Find what object.h transitively pulls in
cpp -dM src/3d/object.h | wc -l
```

### 2. Use forward declarations in headers
**Pattern**:
```cpp
// BAD - pulls in entire header
#include "camera.h"

// GOOD - forward declaration
class Camera;
```

### 3. Move inline functions to .cc files
**Problem**: Inline functions in headers force recompilation of all users when implementation changes.

**Solution**: Move non-trivial inline functions to `.cc` files.

---

## Risks & Mitigation

### Risk 1: Breaking Existing Code
**Likelihood**: Low (forward declaration already exists)

**Mitigation**:
- Compile after each file change
- Run full test suite before committing

### Risk 2: Increased Include Complexity
**Likelihood**: Low (only 3 headers instead of 1)

**Mitigation**:
- Clear naming: `_fwd.h` = forward decls, `_helpers.h` = specialized
- Document in CONTRIBUTING.md

### Risk 3: Forgetting to Include Helpers
**Likelihood**: Medium (compiler errors if you use TextureAsset without including helpers)

**Mitigation**:
- Compiler will error immediately with clear message
- Easy to fix: add `#include "asset_manager_helpers.h"`

---

## Comparison with Original Proposal

### Original Proposal 1 (Asset File Dependencies)
- **Impact**: Fixes correctness bug for asset changes
- **Performance**: No improvement (assets change rarely)
- **Recommendation**: LOW PRIORITY (assets rebuild is acceptable)

### NEW Proposal (Split asset_manager.h)
- **Impact**: 69% faster rebuilds during core library iteration
- **Performance**: 4.82s → 1.5s (developer pain point!)
- **Recommendation**: HIGH PRIORITY ⭐

---

## Measurement Plan

### Before Implementation
```bash
# Baseline: Current rebuild time
time (touch src/util/asset_manager.h && cmake --build build --target demo64k -j8)
# Expected: 4.82s, 35 files
```

### After Implementation
```bash
# Test 1: Forward declaration change (should be instant)
time (touch src/util/asset_manager_fwd.h && cmake --build build --target demo64k -j8)
# Expected: 0.2s, 0 files

# Test 2: Core API change (should be fast)
time (touch src/util/asset_manager.h && cmake --build build --target demo64k -j8)
# Expected: 1.5s, 10 files

# Test 3: Helper change (should be fast)
time (touch src/util/asset_manager_helpers.h && cmake --build build --target demo64k -j8)
# Expected: 1.0s, 7 files
```

---

## Other Headers to Investigate

### Candidates for Similar Treatment
```
mini_math.h    → 1.9s rebuild (23 files) - already isolated to 3d library ✅
camera.h       → unknown (need measurement)
scene.h        → unknown (need measurement)
```

**Recommendation**: Start with `asset_manager.h` (proven 4.8s bottleneck), then measure others if iteration is still slow.

---

## Final Recommendation

**Implement the header split immediately.** This is a high-impact, low-risk change that directly addresses the developer pain point (slow iteration during core library changes).

**Do NOT implement asset file tracking** (Proposal 1 from original doc) - assets change infrequently, and 3.5s rebuild is acceptable.

**Expected Result**: Developer iteration cycle improves from 4.8s → 1.5s (69% faster) for the most common workflow (tweaking core library code).