Opaque: Add support for mutable byte buffers and arrays #155
Conversation
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
We may frequently compare Opaques via HashMap. Cache the #hashCode computation once computed. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
Currently, Opaque is merely a wrapper around byte[], which adds hashCode/equals functionality, preventing mutations, so we can use it as a key in a HashMap. When we process a byte stream (from a ByteBuffer), we may want to look up values from a HashMap. However, we would still need to create new byte[]-backed Opaque objects for the lookup. This creates unnecessary allocations. Improve the Opaque interface to clarify that by itself, Opaques can be mutable. Provide a #toImmutableOpaque() method which is used when storing values in a Map. Adjust FileTracker to use immutable Opaques when storing in a Map. Provide a mutable Opaque implementation backed by a ByteBuffer, and improve equals() for OpaqueImpl. Add a small test case to ensure sanity. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
The SimpleLm keeps all data in memory; there is no obvious need to store the keys as Base64 strings. Use Opaque directly, ensuring that only immutable Opaque keys are used for storing. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
Similar to mutable ByteBuffers, provide support for mutable byte[] backing Opaque. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
... demonstrating the difference between mutable and immutable Opaques. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
kohlschuetter
changed the title
That way, we don't have to declare the cache attributes for hashCode and base64 when not required. Related: dCache#149 Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
kofemann
reviewed
Jul 1, 2025
| OpaqueBufferImpl other = (OpaqueBufferImpl) o; | ||
| ByteBuffer otherBuf = other.buf; | ||
| int otherIndex = other.index; | ||
| for (int i = index, n = index + length, oi = otherIndex; i < n; i++, oi++) { |
There was a problem hiding this comment.
you can use ByteBuffer#mismatch, which is based on vector operations
There was a problem hiding this comment.
Unfortunately, ByteBuffer#mismatch starts checking at position(), which is not what we want (the underlying code would support that but it's not exposed).
It's actually also unlikely that we'll compare two buffer-backed Opaques. It's usually a buffer-backed opaque and an immutable byte[]-backed one in a HashMap.
The general use case is that we can read an entire message into a ByteBuffer, and then look up what we have in map without any further allocation.
kohlschuetter
commented
Jul 1, 2025
| if (o instanceof OpaqueImpl) { | ||
| return Arrays.equals(_opaque, ((OpaqueImpl) o)._opaque); | ||
| } else if (o instanceof OpaqueBufferImpl) { | ||
| OpaqueBufferImpl other = (OpaqueBufferImpl) o; |
There was a problem hiding this comment.
The general use case is that we can read an entire message into a ByteBuffer, and then look up what we have in map without any further allocation.
Compared to the original code of Opaque the "semantics" are unchanged (since Opaque was mutable).
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.
In an effort to further reduce the number of allocations, add support for Opaques backed by
ByteBufferorbyte[]instances that may be mutable. Adjust theOpaqueAPI to allow these mutable objects to be used in the context of hashmaps for reads, as well as for writes (by converting them to an immutable variant).Also cache the
hashCodevalue for immutable Opaques, which further reduces the computational overhead, and allows them to be used directly as keys in LockManager (see the change for SimpleLm).