refactor: change magic finder to stack buffer#763
Conversation
There was a problem hiding this comment.
The changes look good overall. This PR successfully optimizes the MagicFinder by specializing it to work with 4-byte magic signatures and replacing heap allocation with stack allocation. The implementation is clean and consistent across both modified files.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
There was a problem hiding this comment.
Code Review
This pull request refactors MagicFinder and OptimisticMagicFinder to use fixed-size stack buffers and a defined MagicSize type, replacing heap-allocated slices and improving memory locality. Visibility for these internal components has been restricted to pub(crate). Feedback suggests considering MaybeUninit for the 2KB buffer in MagicFinder to avoid the performance cost of zero-initialization.
| ) -> Self { | ||
| Self { | ||
| buffer: vec![0; BUFFER_SIZE].into_boxed_slice(), | ||
| buffer: [0; BUFFER_SIZE], |
|
CI is currently blocked by rust-lang/libz-sys#265 |
|
Currently this MR also reduces the BUFFER size to 1024 instead of 2048. 2048 would only be useful if there is a comment I think |
Pr0methean
left a comment
There was a problem hiding this comment.
Mostly looks good, but a couple of nits.
The PR title must conform to Conventional Commits
specialize our MagicFinder to 4
use a stack array instead of a Box<[u8]>
reduce the BUFFER size to 1024 instead of 2048.