Bill commented on code in PR #7621:
URL: https://github.com/apache/geode/pull/7621#discussion_r858988486
##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -394,4 +370,36 @@ public ByteBuffer getBB() {
}
}
+ public static class PooledByteBuffer {
Review Comment:
At first I was gonna ask you to put `PooledByteBuffer` in it's own file, but
now that I see it's so tiny, I think it's ok as is.
##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -394,4 +370,36 @@ public ByteBuffer getBB() {
}
}
+ public static class PooledByteBuffer {
+ private final ByteBuffer original;
+ private final ByteBuffer byteBuffer;
Review Comment:
These field names are ok but they would make more sense to me as
`poolableBuffer` and `usableSlice`.
**But I will approve this PR as-is if you don't like this suggestion.**
If you like this idea you might continue to call the accessor returning
`slice`: `getBuffer()`. The mismatch is just fine since the field name is an
internal detail, the accessor is for users of the object. Most users of this
class (besides `BufferPool`) don't care that they are getting a slice.
I'd change `getByteBuffer()` to be `getPoolableBuffer()`.
##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -394,4 +370,36 @@ public ByteBuffer getBB() {
}
}
+ public static class PooledByteBuffer {
+ private final ByteBuffer original;
+ private final ByteBuffer byteBuffer;
+
+ PooledByteBuffer(ByteBuffer original, ByteBuffer byteBuffer) {
+ this.original = original;
+ this.byteBuffer = byteBuffer;
+ }
+
+ PooledByteBuffer(ByteBuffer byteBuffer) {
+ this(byteBuffer, byteBuffer);
+ }
+
+ /**
+ * Returns the byte buffer intended for use outside of the pool.
+ */
+ public ByteBuffer getByteBuffer() {
+ return byteBuffer;
+ }
+
+ /**
+ * The original byte buffer managed by the pool.
+ * This should only be used by the pool itself.
+ * It may differ from 'byteBuffer' since it may be a slice of the original.
Review Comment:
Please clarify the second sentence. The two "it"s in this sentence refer to
different things so the sentence is hard to understand. The first "it" refers
to `original`. The sentence only makes sense if the second "it" refers to
`byteBuffer`.
We might say:
"'byteBuffer' may be a slice of 'original'."
##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -394,4 +370,36 @@ public ByteBuffer getBB() {
}
}
+ public static class PooledByteBuffer {
+ private final ByteBuffer original;
+ private final ByteBuffer byteBuffer;
+
+ PooledByteBuffer(ByteBuffer original, ByteBuffer byteBuffer) {
Review Comment:
Good. I verified the only place this is being called outside the class is in
the one place the pool slices a buffer before returning it.
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java:
##########
@@ -29,7 +29,6 @@
public class MemberJvmOptions {
static final int CMS_INITIAL_OCCUPANCY_FRACTION = 60;
private static final List<String> JAVA_11_OPTIONS = Arrays.asList(
- "--add-exports=java.base/sun.nio.ch=ALL-UNNAMED",
Review Comment:
Yay!
##########
geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptionsTest.java:
##########
@@ -43,7 +43,6 @@ void java8Options() {
@EnabledForJreRange(min = JRE.JAVA_11)
void java11Options() {
List<String> expectedOptions = asList(
- "--add-exports=java.base/sun.nio.ch=ALL-UNNAMED",
Review Comment:
🐳 done! (well done)
--
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]
