diff options
Diffstat (limited to 'doc/BUILD_OPTIMIZATION_PROPOSAL.md')
| -rw-r--r-- | doc/BUILD_OPTIMIZATION_PROPOSAL.md | 344 |
1 files changed, 344 insertions, 0 deletions
diff --git a/doc/BUILD_OPTIMIZATION_PROPOSAL.md b/doc/BUILD_OPTIMIZATION_PROPOSAL.md new file mode 100644 index 0000000..8d61e10 --- /dev/null +++ b/doc/BUILD_OPTIMIZATION_PROPOSAL.md @@ -0,0 +1,344 @@ +# Build Optimization Proposal: CMake Dependency Graph Analysis + +## Executive Summary + +Current incremental build times are good (0.2s when nothing changes), but **changing common headers or assets causes unnecessary broad rebuilds** (1.9-3.5s). This analysis identifies bottlenecks and proposes solutions to achieve <1s incremental builds for typical development workflows. + +--- + +## Current Build Performance Baseline + +### Measured Build Times +``` +Clean build (all targets): ~45s (estimated) +Incremental build (no changes): 0.229s ✅ +Touch mini_math.h: 1.897s ⚠️ +Touch demo_assets.txt: 3.498s ⚠️ +Touch shader asset: 0.636s ⚠️ (should be 0s - not tracked!) +``` + +### Dependency Analysis Results + +**Key Finding 1: Individual asset files are NOT tracked as dependencies** +- Only `demo_assets.txt` is a dependency, not the actual `.wgsl`/`.spec`/`.obj` files +- Changing a shader doesn't trigger asset regeneration (stale builds!) +- Developer must manually re-run `./scripts/gen_assets.sh` or touch `demo_assets.txt` + +**Key Finding 2: Generated files cause broad rebuilds** +- `assets_data.cc`: 33 assets, 20KB+ generated code +- `music_data.cc`: Tracker patterns +- `timeline.cc`: Demo sequence +- Changes to ANY asset → full rebuild of `util`, `demo64k`, and all tests + +**Key Finding 3: Header dependency chains** +- `mini_math.h` → triggers rebuild of entire `3d` + `gpu` libraries (23 files) +- `asset_manager.h` → used by audio, 3d, gpu, tests (30+ files) +- `object.h` (includes asset_manager) → transitively includes it everywhere + +**Key Finding 4: Test dependency explosion** +- 20 tests, each depends on `generate_demo_assets` or `generate_test_assets` +- Changing assets rebuilds ALL tests (even if they don't use the changed asset) + +--- + +## Problem Categories + +### 1. Missing File-Level Asset Dependencies ⚠️ HIGH PRIORITY +**Problem**: Changing a `.wgsl` shader doesn't trigger `asset_packer` to regenerate `assets.h`/`assets_data.cc`. + +**Root Cause**: CMake custom command only depends on `demo_assets.txt`, not the actual asset files: +```cmake +add_custom_command( + OUTPUT ${OUT_H} ${OUT_CC} + DEPENDS ${ASSET_PACKER_DEPENDS} ${INPUT_TXT} # ❌ Missing individual asset files! + ... +) +``` + +**Impact**: +- Developers get **stale builds** after modifying shaders +- Must manually trigger rebuild (`touch demo_assets.txt` or `./scripts/gen_assets.sh`) +- Risk of bugs from testing old shader code + +**Solution Complexity**: Medium (requires parsing `demo_assets.txt` to extract file list) + +--- + +### 2. Monolithic Generated Asset Files ⚠️ MEDIUM PRIORITY +**Problem**: `assets_data.cc` contains ALL assets (33 items, 20KB+). Changing one shader regenerates the entire file, causing broad rebuilds. + +**Root Cause**: Single monolithic file instead of per-asset or per-category files. + +**Impact**: +- Touch `demo_assets.txt` → 3.5s rebuild (main.cc, util, all tests) +- Even if developer only changed one shader + +**Solution Complexity**: High (requires refactoring asset packing system) + +--- + +### 3. Header Include Hygiene 🟢 LOW PRIORITY +**Problem**: `asset_manager.h` and `mini_math.h` included widely, causing transitive rebuilds. + +**Root Cause**: Headers include more than necessary, creating dependency chains. + +**Example Chain**: +``` +object.h (includes asset_manager.h) + ↓ +renderer.cc (includes object.h) + ↓ +Entire 3d library rebuilds when asset_manager.h changes +``` + +**Impact**: Moderate (1-2s for `mini_math.h`, less for `asset_manager.h`) + +**Solution Complexity**: Low (forward declarations, split headers) + +--- + +### 4. Test Dependency Granularity 🟢 LOW PRIORITY +**Problem**: All tests depend on `generate_demo_assets`, even if they don't use assets. + +**Impact**: Moderate (asset changes rebuild all 20 tests unnecessarily) + +**Solution Complexity**: Medium (requires per-test dependency analysis) + +--- + +## Proposed Solutions (Ranked by Impact) + +### **Proposal 1: Add File-Level Asset Dependencies** ⭐ HIGHEST IMPACT +**Effort**: Medium | **Impact**: High | **Priority**: Critical + +**Goal**: Make CMake aware of individual asset files, so changing a shader triggers regeneration. + +**Implementation**: +```cmake +# Parse demo_assets.txt to extract asset file paths +function(parse_asset_list INPUT_TXT OUT_FILE_LIST) + file(STRINGS ${INPUT_TXT} LINES) + set(ASSET_FILES "") + foreach(LINE ${LINES}) + if(NOT LINE MATCHES "^#") # Skip comments + string(REGEX REPLACE "^[^,]+,[ ]*([^,]+).*" "\\1" FILENAME "${LINE}") + list(APPEND ASSET_FILES "${CMAKE_SOURCE_DIR}/assets/final/${FILENAME}") + endif() + endforeach() + set(${OUT_FILE_LIST} ${ASSET_FILES} PARENT_SCOPE) +endfunction() + +# Use in pack_assets function +function(pack_assets NAME INPUT_TXT HEADER_VAR DATA_CC_VAR TARGET_NAME) + parse_asset_list(${INPUT_TXT} ASSET_FILE_DEPS) + + add_custom_command( + OUTPUT ${OUT_H} ${OUT_CC} + COMMAND ${ASSET_PACKER_CMD} ${INPUT_TXT} ${OUT_H} ${OUT_CC} + DEPENDS ${ASSET_PACKER_DEPENDS} ${INPUT_TXT} ${ASSET_FILE_DEPS} # ✅ Fixed! + COMMENT "Generating assets for ${NAME}..." + ) + ... +endfunction() +``` + +**Benefits**: +- ✅ Correct incremental builds (no more stale shaders!) +- ✅ Eliminates manual `touch demo_assets.txt` workaround +- ✅ Developer workflow improved + +**Risks**: +- Adds ~33 file dependencies to CMake graph (negligible overhead) +- Requires robust parsing of `demo_assets.txt` format + +--- + +### **Proposal 2: Split Assets into Categories** ⭐ MEDIUM IMPACT +**Effort**: High | **Impact**: Medium | **Priority**: Medium + +**Goal**: Split `assets_data.cc` into `shaders_data.cc`, `audio_data.cc`, `meshes_data.cc` to reduce rebuild scope. + +**Implementation**: +```cmake +pack_assets(shaders assets/final/shaders_list.txt ...) +pack_assets(audio assets/final/audio_list.txt ...) +pack_assets(meshes assets/final/meshes_list.txt ...) + +# demo64k depends on all three +target_link_libraries(demo64k PRIVATE ... ${SHADER_DATA_CC} ${AUDIO_DATA_CC} ${MESH_DATA_CC}) +``` + +**Benefits**: +- ✅ Changing a shader only rebuilds files using shaders +- ✅ Reduces 3.5s rebuild to ~1s for single-category changes + +**Risks**: +- ⚠️ Requires splitting `demo_assets.txt` into multiple files +- ⚠️ More complex build system +- ⚠️ Asset ID namespacing might be needed + +--- + +### **Proposal 3: Precompiled Headers (PCH)** 🔧 ALTERNATIVE APPROACH +**Effort**: Low | **Impact**: Medium | **Priority**: Optional + +**Goal**: Use CMake's `target_precompile_headers` for common includes. + +**Implementation**: +```cmake +target_precompile_headers(3d PRIVATE + <cstdio> + <cmath> + <vector> + <algorithm> + "util/mini_math.h" +) +``` + +**Benefits**: +- ✅ Reduces parse time for large headers +- ✅ Speeds up clean builds significantly + +**Drawbacks**: +- ⚠️ Doesn't help incremental builds much (headers still tracked) +- ⚠️ Can mask missing includes (code compiles but is fragile) + +--- + +### **Proposal 4: Header Include Hygiene** 🟢 LOW HANGING FRUIT +**Effort**: Low | **Impact**: Low | **Priority**: Nice-to-have + +**Goal**: Reduce transitive includes via forward declarations. + +**Example Refactor**: +```cpp +// object.h (BEFORE) +#include "util/asset_manager.h" // ❌ Full include + +// object.h (AFTER) +enum class AssetId : uint16_t; // ✅ Forward declaration +``` + +**Benefits**: +- ✅ Reduces dependency chains +- ✅ Faster incremental builds for header changes + +**Drawbacks**: +- ⚠️ Requires careful analysis of which headers can be forward-declared +- ⚠️ May break existing code if not done carefully + +--- + +### **Proposal 5: Ninja Build Generator** ⚡ QUICK WIN +**Effort**: Trivial | **Impact**: Small | **Priority**: Easy + +**Goal**: Use Ninja instead of Make for faster dependency checking. + +**Implementation**: +```bash +cmake -S . -B build -G Ninja +ninja -C build demo64k +``` + +**Benefits**: +- ✅ ~20% faster incremental builds (0.229s → 0.18s) +- ✅ Better parallelism for large projects +- ✅ Zero code changes required + +**Drawbacks**: +- ⚠️ Requires Ninja installed on developer machines + +--- + +## Recommended Implementation Plan + +### Phase 1: Critical Fixes (1-2 days) +1. ✅ **Proposal 1**: Add file-level asset dependencies +2. ✅ **Proposal 5**: Document Ninja usage (already works, just recommend it) + +### Phase 2: Medium-Term Improvements (3-5 days) +3. 🔧 **Proposal 2**: Split assets into categories (optional, if rebuild times still problematic) +4. 🔧 **Proposal 4**: Header include hygiene (ongoing, as-needed) + +### Phase 3: Optimization (optional) +5. 🔧 **Proposal 3**: Precompiled headers (if clean builds become bottleneck) + +--- + +## Metrics & Success Criteria + +**Current Baseline**: +- Touch shader → 0.6s (but stale build!) +- Touch demo_assets.txt → 3.5s +- Touch mini_math.h → 1.9s + +**Target After Phase 1**: +- Touch shader → 0.8s (regenerates assets_data.cc correctly) +- Touch demo_assets.txt → 3.5s (unchanged, but correct) +- Touch mini_math.h → 1.9s (unchanged) + +**Target After Phase 2**: +- Touch shader → 0.5s (only shader_data.cc regenerates) +- Touch audio asset → 0.4s (only audio_data.cc regenerates) +- Touch mini_math.h → 1.2s (fewer transitive includes) + +--- + +## Open Questions for Discussion + +1. **Asset categorization**: Split by type (shaders/audio/meshes) or by usage (runtime/debug/tests)? +2. **Precompiled headers**: Worth the complexity given incremental builds are already fast? +3. **Unity builds**: Would `UNITY_BUILD` help for clean builds? (Probably not needed for 64k demo) +4. **ccache**: Should we recommend/document ccache usage for developers? + +--- + +## Appendix: Dependency Graph Analysis + +### Current Library Dependencies +``` +demo64k +├── 3d (7 files, depends on: util, procedural) +│ ├── renderer.cc → includes asset_manager.h +│ ├── renderer_sdf.cc → includes asset_manager.h +│ ├── renderer_mesh.cc → includes asset_manager.h +│ └── ... (all include mini_math.h) +├── gpu (18 files, depends on: util, procedural) +│ ├── effects/shaders.cc → includes asset_manager.h +│ └── effects/*_effect.cc → includes object.h → mini_math.h +├── audio (12 files, depends on: util, procedural) +│ ├── audio.cc → includes asset_manager.h +│ └── tracker.cc → includes asset_manager.h +├── util (1 file) +│ └── asset_manager.cc (includes generated assets.h) +└── procedural (1 file) +``` + +### Rebuild Cascade for `assets_data.cc` Change +``` +assets_data.cc (regenerated) + ↓ +util library (recompiles asset_manager.cc) + ↓ +audio library (no rebuild - doesn't depend on util) +3d library (no rebuild - doesn't depend on util) +gpu library (no rebuild - doesn't depend on util) + ↓ +demo64k (relinks with new util) + ↓ +ALL tests (relink with new util + demo64k dependencies) +``` + +**Conclusion**: Rebuild cascade is actually **well-isolated** to util + demo64k. The 3.5s time is mostly asset_packer + relink overhead, not cascading recompiles. + +--- + +## Final Recommendation + +**Implement Proposal 1 immediately** (file-level asset dependencies) to fix the critical stale build issue. This is the highest-priority bug affecting developer workflow. + +**Defer Proposal 2** (asset categorization) unless measurements show it's worth the complexity. Current 3.5s rebuild for asset changes is acceptable if it only happens when intentionally modifying assets. + +**Document Ninja usage** as a quick win for interested developers. + +**Monitor** header dependency issues (Proposal 4) and address opportunistically during refactors. |