adelapena commented on code in PR #1723:
URL: https://github.com/apache/cassandra/pull/1723#discussion_r969361845
##########
src/java/org/apache/cassandra/db/AbstractBufferClusteringPrefix.java:
##########
@@ -42,10 +42,18 @@ public ByteBuffer[] getBufferArray()
return getRawValues();
}
- public ClusteringPrefix<ByteBuffer> minimize()
+ public ClusteringPrefix<ByteBuffer> retainable()
{
if (!ByteBufferUtil.canMinimize(values))
return this;
- return new BufferClustering(ByteBufferUtil.minimizeBuffers(values));
+
+ ByteBuffer[] values = ByteBufferUtil.minimizeBuffers(this.values);
Review Comment:
Maybe we could use a new name:
```suggestion
ByteBuffer[] minimizedValues =
ByteBufferUtil.minimizeBuffers(values);
```
##########
src/java/org/apache/cassandra/utils/ByteBufferUtil.java:
##########
@@ -681,13 +681,13 @@ public static boolean isPrefix(ByteBuffer prefix,
ByteBuffer value)
public static boolean canMinimize(ByteBuffer buf)
{
- return buf != null && (buf.capacity() > buf.remaining() ||
!buf.hasArray());
+ return buf != null && (!buf.hasArray() || buf.array().length >
buf.remaining());
Review Comment:
Changing from `ByteBuffer#capacity()` to `ByteBuffer#array()` makes it
possible to get `UnsupportedOperationException` and `ReadOnlyBufferException`
if the byte buffer is not backed by a writable array. If we need the change we
should indicate this on the JavaDoc for the method.
##########
src/java/org/apache/cassandra/utils/ByteBufferUtil.java:
##########
@@ -681,13 +681,13 @@ public static boolean isPrefix(ByteBuffer prefix,
ByteBuffer value)
public static boolean canMinimize(ByteBuffer buf)
{
- return buf != null && (buf.capacity() > buf.remaining() ||
!buf.hasArray());
+ return buf != null && (!buf.hasArray() || buf.array().length >
buf.remaining());
}
/** trims size of bytebuffer to exactly number of bytes in it, to do not
hold too much memory */
public static ByteBuffer minimalBufferFor(ByteBuffer buf)
{
- return buf.capacity() > buf.remaining() || !buf.hasArray() ?
ByteBuffer.wrap(getArray(buf)) : buf;
+ return !buf.hasArray() || buf.array().length > buf.remaining() ?
ByteBuffer.wrap(getArray(buf)) : buf;
Review Comment:
Same as in the comment for `canMinimize`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
