|
1 | | -## 2026-02-25 — Review of: Optimize DataHasher::update buffer allocation |
| 1 | +## 2026-02-25 — Generalize video API to accept borrowed slices |
2 | 2 |
|
3 | | -**Verdict:** PASS |
| 3 | +**Done:** Changed `soft_hash_video_v0` and `gen_video_code_v0` from concrete `&[Vec<i32>]` to |
| 4 | +generic `<S: AsRef<[i32]> + Ord>` parameters. Updated both FFI wrappers to construct `Vec<&[i32]>` |
| 5 | +(borrowed slices) instead of `Vec<Vec<i32>>` (heap-allocated copies), eliminating per-frame |
| 6 | +allocations. |
4 | 7 |
|
5 | | -**Summary:** Replaced per-call heap allocations in `DataHasher::update` with a persistent |
6 | | -`buf: Vec<u8>` that is reused across calls. The `data.to_vec()`, `[tail, data].concat()`, and |
7 | | -`prev_chunk.to_vec()` patterns are eliminated — new data is appended via `extend_from_slice`, and |
8 | | -the tail is shifted to the front with `copy_within` + `truncate`. A Criterion streaming benchmark |
9 | | -was added. All 261 tests pass, clippy clean, 14 pre-commit hooks pass, benchmark runs at ~1.0 GiB/s. |
| 8 | +**Files changed:** |
10 | 9 |
|
11 | | -**Verification:** |
12 | | - |
13 | | -- [x] `cargo test -p iscc-lib` passes — 261 tests (208 + 31 + 22), 0 failures |
14 | | -- [x] `cargo clippy -p iscc-lib -- -D warnings` clean |
15 | | -- [x] `grep -c 'to_vec\|\.concat()' crates/iscc-lib/src/streaming.rs` returns 0 — no per-call |
16 | | - allocations remain |
17 | | -- [x] `cargo bench -p iscc-lib -- DataHasher` runs successfully — ~1.0 GiB/s throughput |
18 | | - |
19 | | -**Issues found:** |
| 10 | +- `crates/iscc-lib/src/lib.rs`: Generalized `soft_hash_video_v0` and `gen_video_code_v0` signatures |
| 11 | + to `<S: AsRef<[i32]> + Ord>`. Updated body to use `.as_ref()` for column count and iteration, |
| 12 | + and `BTreeSet<&S>` for deduplication. |
| 13 | +- `crates/iscc-ffi/src/lib.rs`: Changed both `iscc_gen_video_code_v0` and `iscc_soft_hash_video_v0` |
| 14 | + from `Vec<Vec<i32>>` with `.to_vec()` to `Vec<&[i32]>` with direct slice borrows. |
20 | 15 |
|
21 | | -- (none) |
22 | | - |
23 | | -**Next:** The `[normal]` DataHasher issue is resolved. Consider the `[normal]` iscc-ffi video frame |
24 | | -allocation issue (changing `iscc_lib` video API to accept `&[&[i32]]`), or one of the `[low]` issues |
25 | | -(dct power-of-two validation, wtahash bounds check, iscc-py `__version__`, etc.). |
| 16 | +**Verification:** |
26 | 17 |
|
27 | | -**Notes:** The borrow checker pattern (extract `tail_len` as `usize` before `drop(chunks)`) is clean |
28 | | -and well-documented with inline comments. The `drop(chunks)` is explicit to make the borrow release |
29 | | -obvious — good practice for code that manipulates borrowed data and the owning buffer in sequence. |
| 18 | +- `cargo test -p iscc-lib`: 261 tests passed (208 + 31 + 22), 0 failures |
| 19 | +- `cargo test -p iscc-ffi`: 62 tests passed, 0 failures |
| 20 | +- `cargo clippy --workspace --all-targets -- -D warnings`: clean (no warnings) |
| 21 | +- `.to_vec()` count in `crates/iscc-ffi/src/lib.rs`: 1 (only the unrelated `alg_cdc_chunks` on line |
| 22 | + 906 remains — down from 3) |
| 23 | +- `mise run check`: all 14 pre-commit hooks passed |
| 24 | + |
| 25 | +**Next:** The `[normal]` iscc-ffi video frame allocation issue is resolved. Consider the remaining |
| 26 | +`[low]` issues: `alg_dct` power-of-two validation, `alg_wtahash` bounds check, iscc-py |
| 27 | +`__version__`, iscc-py module docstring, iscc-wasm conformance feature gate, or iscc-wasm stale |
| 28 | +CLAUDE.md. |
| 29 | + |
| 30 | +**Notes:** The generic approach (`AsRef<[i32]> + Ord`) was chosen over concrete `&[&[i32]]` to |
| 31 | +maintain backward compatibility — all existing callers that pass `&[Vec<i32>]` compile unchanged |
| 32 | +because `Vec<i32>` implements both traits. Only the FFI crate (the actual beneficiary) was modified. |
| 33 | +No other binding crate (py, napi, wasm, jni) required changes. |
0 commit comments