Fix errors in Mapped pool and javadoc #8264
Conversation
Signed-off-by: Greg Wilkins <[email protected]>
|
Also some real fixes now! |
| @@ -302,4 +308,30 @@ public String toString() | |||
| getMaxBucketSize(), | |||
| getCapacityFactor()); | |||
| } | |||
|
|
|||
| protected class Retained extends ArrayRetainableByteBufferPool | |||
There was a problem hiding this comment.
Seems strange to me that you would create a MappedByteBufferPool and ask it for its RetainableBufferPool and it gives you an array based implementation.
There was a problem hiding this comment.
yeah, but not sure we want to create a MappedRetainableByteBuffer pool. Prior to the combination of pool, this is exactly what we would have got if the user selected a MBBP, as we would have created an ARBBP. So just a bit strange that they are together.
I'm not actually sure if we really want to keep MBBP. @sbordet @lorban why do we have it?
There was a problem hiding this comment.
AFAICT it's only used in the client, but I can't see why MBBP was created in the first place and I lack history. So unless @sbordet has a good reason to keep it, that pool impl can go IMHO.
There was a problem hiding this comment.
OK, but for this PR, I'd like to fix javadoc and MBBP to unblock other work.
We can consider deprecating MBBP later.
There was a problem hiding this comment.
Sounds good to me, and the change looks reasonable enough: currently MBBP is used together with a ARBBP so this change just solidifies a behavior that already exist.
There was a problem hiding this comment.
@lorban can I get a green tick then please?
|
@lachlan-roberts @lorban Can I get this approved? |
I just noticed this was wrong with our buffer pool javadoc
Signed-off-by: Greg Wilkins [email protected]