chore: start refactoring to only use bytes by refactoring the ByteBuffer internals#47
Merged
CommanderStorm merged 15 commits intofast-pack:mainfrom Feb 13, 2026
Merged
Conversation
CommanderStorm
requested review from
Copilot and
nyurik
and removed request for
Copilot
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the ByteBuffer implementation to use the bytes crate instead of a custom buffer implementation, simplifying the codebase by removing the custom IntBuffer abstraction and associated methods like flip() and as_int_buffer().
Changes:
- Replaced custom
ByteBufferimplementation withBytesMutfrom thebytescrate - Removed
IntBufferstruct and its associated conversion logic - Updated
variable_byte.rsandfastpfor.rsto use direct iteration withget_u32_le()andput_u32_le()instead of buffer conversion methods
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/rust/bytebuffer.rs | Replaced custom buffer implementation with BytesMut, removed IntBuffer and buffer manipulation methods |
| src/rust/integer_compression/variable_byte.rs | Refactored to use iterator-based approach with get_u32_le() instead of as_int_buffer() |
| src/rust/integer_compression/fastpfor.rs | Updated to use direct loops with get_u32_le() and put_u32_le() instead of buffer conversion |
| Cargo.toml | Added bytes dependency to the rust feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CommanderStorm
changed the title
buffer by refactoring the ByteBuffer internalsbytes by refactoring the ByteBuffer internals
for more information, see https://pre-commit.ci
nyurik
approved these changes
Feb 13, 2026
for more information, see https://pre-commit.ci
…ly is (fast-pack#50) this is very hard to read otherwise
one of the problems with this crate is that it is currently fairly dense and sparsely documented. Disclaimer: I let an LLM do the first pass, but verified every documented claim --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… geometric data for compression/decompression/round-trip/rate (fast-pack#51) I asked my LLM (claude) of choice to write a benchmark for this since we don't currently have one and since this is fairly manual work. This is mostly done to check a few assumptions around fast-pack#49 I reviewed every line and fixed the issues that were caused. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
nyurik
approved these changes
Feb 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this is the first of many PR, otherwise I cannot make this reviewable.
I am a abit tiread, will go over my changes tomorrow again
Works towards #35