Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

borrow primitive types where possible instead of allocating them on the heap in BytesEncode implementations #272

Closed

Conversation

antonilol
Copy link
Contributor

Pull Request

Related issue

somewhat related to #257

What does this PR do?

Remove unnecessary allocations (heap allocations created with the vec![...] macro) in implementations of BytesEncode. Not all could be removed, without breaking the api, bytes need to be copied if they need to be flipped around (when using big endian integers on little endian machines).

@Kerollmops
Copy link
Member

Hey @antonilol 👋

Thank you for the great work but I am not ready to merge all of these unsafe. I would rather you propose a trait change by following the #257 instructions and my comment (to declare a BytesEncode::zero_copy method so that it's dynamic).

What do you think? We will then test that in Meilisearch to see the speed gain.

@Kerollmops Kerollmops closed this Aug 17, 2024
@antonilol
Copy link
Contributor Author

I am currently working on a new trait that is also (99%) backwards compatible with BytesEncode, this pr was more the 'quick fix' for what was possible with this current trait. The new one makes no heap allocations for primitive types, for both byte orders. Will make a (draft) pr with the current state of it!

@antonilol antonilol deleted the bytes-encode-less-alloc branch August 17, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants