Conversation
`BitBufferMut::collect_words_in` allocated with `with_capacity`, immediately called `set_len` to claim the memory was initialised, and then handed `as_mut_slice()` to the fill closure. That materialises a `&mut [u64]` over uninitialised memory before anything has written to it. The existing safety comment argued this was acceptable because `fill` writes every word and `u64` has no invalid bit patterns. That is not sufficient: producing a reference to uninitialised memory is Undefined Behaviour in its own right, independent of the validity invariants of the referent type. Use `BufferMut::zeroed`, which returns an allocation whose elements are already initialised and whose length is already set. `fill` still overwrites every word, so the only cost is one memset. Signed-off-by: Jian He <hejia@google.com>
Merging this PR will degrade performance by 17.24%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | from_bool_slice[65536] |
74.6 µs | 97.4 µs | -23.46% |
| ❌ | WallTime | dict_canonicalize_gt_u8_avx512[1000000] |
422.3 µs | 472 µs | -10.52% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing jianhe25:buffer-fix-5 (0f4e9c6) with develop (b127616)
Footnotes
-
503 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
BitBufferMut::collect_words_inallocated withwith_capacity_in, calledset_lento claim the memory was initialised, and passedas_mut_slice()to the fill closure. That creates a&mut [u64]over uninitialised memory before anything writes to it. The safety comment argued this was fine becausefillwrites every word andu64has no invalid bit patterns, but a reference to uninitialised memory is UB on its own.This uses
BufferMut::zeroed_in(num_words, allocator)instead, keeping the allocator.fillstill overwrites every word, so the only extra cost is one memset.Testing
BitBufferMuttests; I didn't add a new one.cargo test -p vortex-buffer --all-features, clippy, nightly fmt, and Miri onbit::buf_mutwith CI'sMIRIFLAGS.AI assistance
This change was prepared with help from an AI coding assistant (Google-internal agentic tooling). I reviewed and verified the change and the tests myself.