dcapwell commented on code in PR #2122:
URL: https://github.com/apache/cassandra/pull/2122#discussion_r1100751739
##########
test/unit/org/apache/cassandra/utils/Generators.java:
##########
@@ -314,11 +324,36 @@ public static Gen<ByteBuffer> bytes(int min, int max)
// to add more randomness, also shift offset in the array so the
same size doesn't yield the same bytes
int offset = (int) rnd.next(Constraint.between(0, MAX_BLOB_LENGTH
- size));
- return ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES, offset, size);
+ return handleCases(cases, rnd, offset, size);
};
+ };
+
+ private enum BBCases { HEAP, READ_ONLY_HEAP, DIRECT, READ_ONLY_DIRECT }
+
+ private static ByteBuffer handleCases(Gen<BBCases> cases, RandomnessSource
rnd, int offset, int size) {
+ switch (cases.generate(rnd))
+ {
+ case HEAP: return ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES,
offset, size);
+ case READ_ONLY_HEAP: return
ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES, offset, size).asReadOnlyBuffer();
+ case DIRECT:
+ {
+ ByteBuffer bb = ByteBuffer.allocateDirect(size);
+ bb.put(LazySharedBlob.SHARED_BYTES, offset, size);
+ bb.flip();
+ return bb;
Review Comment:
when I said you should avoid copy/paste and make this a method, I mean
```
ByteBuffer bb = ByteBuffer.allocateDirect(size);
bb.put(LazySharedBlob.SHARED_BYTES, offset, size);
bb.flip();
```
and not this new `handleCases`...
##########
src/java/org/apache/cassandra/transport/CBUtil.java:
##########
@@ -478,7 +487,32 @@ public static void writeValue(ByteBuffer bytes, ByteBuf cb)
cb.writeInt(remaining);
if (remaining > 0)
- cb.writeBytes(bytes.duplicate());
+ addBytes(bytes, cb);
+ }
+
+ public static void addBytes(ByteBuffer src, ByteBuf dest)
+ {
+ if (src.remaining() == 0)
+ return;
+
+ int length = src.remaining();
+
+ //heap buffers are copied this way in order to avoid JVM crashing
Review Comment:
please give more detail, this isn't enough for anyone to understand why this
code is this way. You should also move it within the `src.hasArray` block as
that comment only makes sense there.
##########
test/unit/org/apache/cassandra/utils/Generators.java:
##########
@@ -314,11 +324,36 @@ public static Gen<ByteBuffer> bytes(int min, int max)
// to add more randomness, also shift offset in the array so the
same size doesn't yield the same bytes
int offset = (int) rnd.next(Constraint.between(0, MAX_BLOB_LENGTH
- size));
- return ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES, offset, size);
+ return handleCases(cases, rnd, offset, size);
};
+ };
+
+ private enum BBCases { HEAP, READ_ONLY_HEAP, DIRECT, READ_ONLY_DIRECT }
+
+ private static ByteBuffer handleCases(Gen<BBCases> cases, RandomnessSource
rnd, int offset, int size) {
+ switch (cases.generate(rnd))
+ {
+ case HEAP: return ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES,
offset, size);
+ case READ_ONLY_HEAP: return
ByteBuffer.wrap(LazySharedBlob.SHARED_BYTES, offset, size).asReadOnlyBuffer();
+ case DIRECT:
+ {
+ ByteBuffer bb = ByteBuffer.allocateDirect(size);
+ bb.put(LazySharedBlob.SHARED_BYTES, offset, size);
+ bb.flip();
+ return bb;
+ }
+ case READ_ONLY_DIRECT:
+ {
+ ByteBuffer bb = ByteBuffer.allocateDirect(size);
+ bb.put(LazySharedBlob.SHARED_BYTES, offset, size);
+ bb.flip();
+ return bb.asReadOnlyBuffer();
+ }
+ default: throw new AssertionError("cann't wait for jdk 17!");
+ }
}
- /**
+ /**
Review Comment:
please revert
--
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]
