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

support bytebuffer updates on streaming hashes #187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pyr
Copy link

@pyr pyr commented Feb 2, 2022

In some cases ByteBuffers do not provide access to an underlying
byte array without incurring copies, this is notably the case
for off-heap direct bytebuffers.

This patch provides a way to call update on streaming XXHASH instances
against a ByteBuffer, which avoids the extra copy needed into a byte
array.

I'm putting this one up to gauge interest. If present, I can adapt the Java safe and unsafe variants to also support the method, I first wanted some feedback on the general idea.

I'm also a bit unclear on what to do should we reach a case where the buffer supports neither .isDirect nor .hasArray, in this case it feels as though the only way forward is to allocate a byte array to copy the ByteBuffer contents into

@pyr pyr force-pushed the streaming-on-bytebuffers branch from 7949a20 to e5e68ba Compare February 2, 2022 10:32
In some cases ByteBuffers do not provide access to an underlying
byte array without incurring copies, this is notably the case
for off-heap direct bytebuffers.

This patch provides a way to call update on streaming XXHASH instances
against a ByteBuffer, which avoids the extra copy needed into a byte
array.
@pyr pyr force-pushed the streaming-on-bytebuffers branch from e5e68ba to f65a4b7 Compare February 2, 2022 10:38
@pyr
Copy link
Author

pyr commented Oct 17, 2023

Hi, a quick update on this one, it has been running fine for over a year in production. Is there any interest in seeing this get in or should I close the PR?

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.

1 participant