hcie-rust-v3.05/.kilo/plans/1784932432251-criterion-merge-and-optimize.md

# Criterion Merge & 2x Optimization Plan

## Context

Two criterion output directories exist:
- **Root `criterion/`** — has HTML report (`report/index.html`) + all 8 `gold_standard` baseline variants
- **`hcie-engine-api/criterion/`** — partial duplicate (4 benchmark dirs, no report, no gold_standard)

Root cause: each benchmark file hardcodes `Criterion::default().output_directory(Path::new("criterion"))` — a relative path that resolves differently depending on CWD. No `.cargo/config.toml` exists to set `CRITERION_HOME`.

Gold standard timings (from root `criterion/`):

| Benchmark | Mean | Dominated by |
|-----------|------|--------------|
| `below_cache_reuse/first_stroke_cache_build` | 0.81 s | Full 9-layer below-cache composite |
| `below_cache_reuse/second_stroke_cache_reuse` | 0.015 s | Cache hit — fast |
| `composite_scratch_pooling/cold_composite` | 1.05 s | Full 10-layer composite + scratch alloc |
| `composite_scratch_pooling/warm_composite` | 0.010 s | Pooled scratch — fast |
| `effects_skip/no_effects_composite` | 0.014 s | Effects skipped — fast |
| `effects_skip/with_one_dropshadow` | 0.015 s | Single effect — fast |
| `stroke_mask_pooling/cold_first_stroke` | 0.046 s | Mask alloc + first stroke |
| `stroke_mask_pooling/warm_10th_stroke` | 0.017 s | Pooled mask — fast |

## Goal

1. Merge criterion directories into single root `criterion/`
2. Fix output path configuration so benchmarks always write to the same place
3. Achieve 2x speedup across all 8 benchmark variants vs `gold_standard` baseline
4. Update `notlar.txt` and `criterion.md` with correct paths

## Task List

### Phase 1: Directory Merge & Path Fix (config-only, no engine changes)

**1.1** Delete `hcie-engine-api/criterion/` directory entirely
```bash
rm -rf hcie-engine-api/criterion/
```

**1.2** Create `.cargo/config.toml` at workspace root:
```toml
[env]
CRITERION_HOME = { value = "criterion", relative = true }
```
This ensures all `cargo bench` invocations write to `./criterion/` relative to workspace root, regardless of CWD.

**1.3** Remove hardcoded `output_directory` from all 4 benchmark files. Change:
```rust
criterion_group!{
    name = benches;
    config = Criterion::default().output_directory(std::path::Path::new("criterion"));
    targets = bench_xxx
}
```
To:
```rust
criterion_group!(benches, bench_xxx);
criterion_main!(benches);
```
Files to edit:
- `hcie-engine-api/benches/stroke_mask_pooling.rs:119-124`
- `hcie-engine-api/benches/composite_scratch_pooling.rs:145-150`
- `hcie-engine-api/benches/below_cache_reuse.rs:145-150`
- `hcie-engine-api/benches/effects_skip.rs:165-170`

**1.4** Remove `bench = false` from `hcie-engine-api/Cargo.toml` `[lib]` section (line 37). This was a workaround; with explicit `[[bench]]` targets it's not needed.

**1.5** Update `.gitignore` — add `criterion/` to prevent benchmark output from being committed (the gold_standard baseline should be kept but the report and run data are generated).

**1.6** Update `notlar.txt`:
- Fix line 19: `CRITERION_HOME=criterion` → reference `.cargo/config.toml`
- Fix line 20: `./criterion/report/index.html` stays correct (now guaranteed by config.toml)
- Add note about `.cargo/config.toml` setting `CRITERION_HOME`

**1.7** Update `criterion.md`:
- Fix line 40: `./criterion/report/index.html` stays correct
- Add note about `.cargo/config.toml` configuration

**1.8** Verify: run `cargo bench -p hcie-engine-api` and confirm output goes to root `criterion/` only.

### Phase 2: Save Fresh Baseline & Verify

**2.1** Save a fresh `gold_standard` baseline after path fixes:
```bash
cargo bench -p hcie-engine-api -- --save-baseline gold_standard
```

**2.2** Run regression comparison to confirm baselines match:
```bash
cargo bench -p hcie-engine-api -- --baseline gold_standard
```

**2.3** Run visual regression and performance tests to confirm no breakage:
```bash
cargo test -p hcie-engine-api --test visual_regression
cargo test -p hcie-engine-api --test performance_stroke_4k -- --nocapture
```

### Phase 3: Optimization Loop

For each optimization, follow this cycle:
1. Unlock target crate: `./unlock.sh <crate>`
2. Apply optimization
3. Build: `cargo build -p hcie-engine-api`
4. Run benchmarks: `cargo bench -p hcie-engine-api -- --baseline gold_standard`
5. Check for regressions: `cargo test -p hcie-engine-api --test visual_regression`
6. If improvement < 2x, continue to next optimization
7. Lock crate: `./lock.sh <crate>`

**Optimization targets (in priority order):**

#### 3.1 Pre-allocate pooled buffers in `Engine::new()` (hcie-engine-api)

Currently `composite_scratch`, `active_stroke_mask`, `below_cache`, and `stroke_before_buf` are all `None` at construction and allocated lazily on first use. The cold benchmarks pay this allocation cost.

Change `Engine::new_with_options()` in `hcie-engine-api/src/lib.rs` to pre-allocate:
- `composite_scratch = Some(vec![0u8; w * h * 4])` — ~33MB
- `active_stroke_mask = Some(vec![0u8; w * h])` — ~8MB  
- `below_cache = Some(vec![0u8; w * h * 4])` — ~33MB
- `stroke_before_buf = Some(vec![0u8; w * h * 4])` — ~33MB

This moves allocation cost from first-stroke time to engine-creation time (which is excluded from benchmark measurement via `iter_with_setup`). Expected impact: cold benchmarks drop significantly because they no longer include allocation.

#### 3.2 Opaque-layer early-exit in composite (hcie-composite, hcie-engine-api)

In `composite_tiled_into` (`hcie-composite/src/tiled.rs`), when compositing layers bottom-to-top, if a layer is:
- Fully opaque (alpha=255 everywhere in the dirty region)
- Normal blend mode
- 100% opacity

...then all layers below it are completely occluded and can be skipped.

Implementation: Before compositing each layer, check if the output buffer is already fully opaque in the dirty region. If so, skip remaining below-layers. This is most impactful for the `first_stroke_cache_build` and `cold_composite` benchmarks where all 10 layers are filled with opaque colors.

#### 3.3 `copy_from_slice` fast-path for opaque Normal layers (hcie-composite)

In the per-pixel blend loop, add a fast-path: if the source pixel is fully opaque (alpha=255) and blend mode is Normal, use `copy_from_slice` for the entire row instead of per-pixel blending. This avoids the blend math for the common case of opaque layers.

#### 3.4 Reduce `Vec::clone()` in hot paths (hcie-engine-api)

In `draw_filled_rect_rgba` (`stroke_brush.rs:582`): `layer.pixels.clone()` creates a full ~33MB copy for undo. Replace with `copy_from_slice` into a pooled buffer.

In `begin_stroke` (`stroke_cache.rs:200-208`): `layer.effects.clone()` and `layer.styles.clone()` are small but unnecessary — effects/styles are typically empty. Add an early-return if both are empty.

#### 3.5 Parallelize `rebuild_below_cache_if_needed` (hcie-engine-api)

The below-cache rebuild in `stroke_cache.rs:514-611` composites all layers below the active one. This is currently sequential. Use Rayon to parallelize the tile compositing within this function, similar to how `render_composite_region` uses `par_chunks_mut`.

#### 3.6 SIMD blend operations (hcie-blend, if needed)

If the above optimizations don't reach 2x, explore SIMD-accelerated blend operations in `hcie-blend`. The `blend_pixel` function is called millions of times per composite. Using `std::simd` or explicit SSE/AVX intrinsics could yield 4-8x speedup on blend operations.

### Phase 4: Final Verification

**4.1** Run full benchmark suite and compare against original gold_standard:
```bash
cargo bench -p hcie-engine-api -- --baseline gold_standard
```

**4.2** Run visual regression tests:
```bash
cargo test -p hcie-engine-api --test visual_regression
cargo test -p hcie-engine-api --test performance_stroke_4k -- --nocapture
```

**4.3** Save final optimized baseline:
```bash
cargo bench -p hcie-engine-api -- --save-baseline gold_standard
```

**4.4** Update `notlar.txt` with final benchmark results.

## Files to Modify

| File | Change |
|------|--------|
| `.cargo/config.toml` | **Create** — set `CRITERION_HOME` |
| `.gitignore` | Add `criterion/` entry |
| `hcie-engine-api/Cargo.toml` | Remove `bench = false` from `[lib]` |
| `hcie-engine-api/benches/stroke_mask_pooling.rs` | Remove `output_directory`, simplify `criterion_group!` |
| `hcie-engine-api/benches/composite_scratch_pooling.rs` | Same |
| `hcie-engine-api/benches/below_cache_reuse.rs` | Same |
| `hcie-engine-api/benches/effects_skip.rs` | Same |
| `notlar.txt` | Update paths and commands |
| `criterion.md` | Update paths |
| `hcie-engine-api/src/lib.rs` | Pre-allocate buffers in `new_with_options()` |
| `hcie-engine-api/src/stroke_cache.rs` | Optimize `begin_stroke`, `rebuild_below_cache_if_needed` |
| `hcie-engine-api/src/stroke_brush.rs` | Optimize `draw_filled_rect_rgba` clone |
| `hcie-composite/src/tiled.rs` | Opaque-layer skip, `copy_from_slice` fast-path |

## Files to Delete

| Path | Reason |
|------|--------|
| `hcie-engine-api/criterion/` | Duplicate — root `criterion/` is canonical |

## Locked Crates That Need Unlocking

| Crate | Reason |
|-------|--------|
| `hcie-engine-api` | Pre-allocation, hot-path optimizations |
| `hcie-composite` | Opaque-layer skip, copy_from_slice fast-path |
| `hcie-blend` | SIMD blend (only if Phase 3.1-3.5 insufficient) |

## Validation

After each phase:
- `cargo build -p hcie-engine-api` must succeed
- `cargo bench -p hcie-engine-api` must produce output in root `criterion/` only
- `cargo test -p hcie-engine-api --test visual_regression` must pass (8/8)
- `cargo test -p hcie-engine-api --test performance_stroke_4k -- --nocapture` must not regress
- `cargo bench -p hcie-engine-api -- --baseline gold_standard` must show improvement (not regression)

## Risks

- **Pre-allocation increases engine memory footprint** by ~107MB (33+8+33+33). This is acceptable for a 4K image editor where the document itself is already ~33MB per layer.
- **Opaque-layer skip changes composite behavior** if a layer has non-Normal blend mode but appears opaque. The check must verify both alpha=255 AND blend mode=Normal.
- **SIMD requires `std::simd` (unstable)** or explicit intrinsics with `cfg(target_feature)` guards. Prefer portable approaches first.