[
https://issues.apache.org/jira/browse/GEODE-6636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16869956#comment-16869956
]
Darrel Schneider commented on GEODE-6636:
-----------------------------------------
I found the following changes cause the size of the buffers in this pool to be
consistent. So consider using it as part of this fix or another ticket:
{noformat}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/net/NioPlainEngine.java
b/geode-core/src/main/java/org/apache/geode/internal/net/NioPlainEngine.java
index 2c901e6836..a10515a0b8 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/net/NioPlainEngine.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/NioPlainEngine.java
@@ -35,9 +35,16 @@ public class NioPlainEngine implements NioFilter {
int lastReadPosition;
int lastProcessedPosition;
+ final int minimumBufferSize;
- public NioPlainEngine() {}
+ public NioPlainEngine() {
+ this(0);
+ }
+
+ public NioPlainEngine(int minimumBufferSize) {
+ this.minimumBufferSize = minimumBufferSize;
+ }
@Override
public ByteBuffer wrap(ByteBuffer buffer) {
@@ -56,7 +63,11 @@ public class NioPlainEngine implements NioFilter {
ByteBuffer buffer = wrappedBuffer;
if (buffer == null) {
- buffer = Buffers.acquireBuffer(bufferType, amount, stats);
+ int bufferSize = amount;
+ if (bufferSize < minimumBufferSize) {
+ bufferSize = minimumBufferSize;
+ }
+ buffer = Buffers.acquireBuffer(bufferType, bufferSize, stats);
buffer.clear();
lastProcessedPosition = 0;
lastReadPosition = 0;
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
index 2ba313ed89..f6533ccf36 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
@@ -1132,7 +1132,9 @@ public class Connection implements Runnable {
if (!sharedResource) {
setReceiveBufferSize(channel.socket(),
this.owner.getConduit().tcpBufferSize);
} else {
- setReceiveBufferSize(channel.socket(), SMALL_BUFFER_SIZE); // make
small since only
+ // prefer standard size buffer
+ setReceiveBufferSize(channel.socket(),
this.owner.getConduit().tcpBufferSize);
+ // setReceiveBufferSize(channel.socket(), SMALL_BUFFER_SIZE); // make
small since only
// receive ack messages
}
setSendBufferSize(channel.socket());
@@ -1853,7 +1855,7 @@ public class Connection implements Runnable {
ioFilter =
getConduit().getSocketCreator().handshakeSSLSocketChannel(channel, engine,
getConduit().idleConnectionTimeout, clientSocket, inputBuffer,
getConduit().getStats());
} else {
- ioFilter = new NioPlainEngine();
+ ioFilter = new NioPlainEngine(getConduit().tcpBufferSize);
}
}
{noformat}
> Buffers.acquireBuffer is not optimal
> ------------------------------------
>
> Key: GEODE-6636
> URL: https://issues.apache.org/jira/browse/GEODE-6636
> Project: Geode
> Issue Type: Improvement
> Components: core
> Reporter: Darrel Schneider
> Assignee: Helena Bales
> Priority: Major
> Labels: performance
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> org.apache.geode.internal.net.Buffers.acquireBuffer takes buffers out of a
> ConcurrentLinkedQueue. If the buffer is too small then it adds it back to the
> queue and adds it to an IdentityHashMap. The map is just to detect if we have
> looped around and found one we added to the map in a previous iteration.
> A more efficient way to do this, which will only remove things from the queue
> that we will either throw away or use and return later, and which gets rid of
> the map, is to use ConcurrentLinkedQueue.remove(Object). You can see an
> example of this by looking at:
> AvailableConnectionManager.EqualsWithPredicate. The predicate to use with
> acquireBuffer is that the soft reference is null or that the capacity of the
> referenced buffer is large enough. If you remove one because the reference is
> null then you need to call remove(Object) again (after decrementing the
> correct stat) since all you did was find one that had been garbage collected.
> You want to keep the predicate as cheap as possible since it is called in the
> "compare-and-set" spin loop. The more you do in the predicate, the more
> likely a concurrent thread will take that buffer and you will need to spin
> around and try again.
> I was surprised when running a benchmark under the profiler to see operations
> on this IdentityHashMap show up. The benchmark was doing pr puts of all the
> same size so I would have thought all these direct buffers would be the same.
> I think it would be worth understanding what sizes of buffers will be
> requested by acquireBuffer and how often they will be acquired and returned.
> If different sizes is a normal use case then it probably would make sense to
> have more than one queue. If we could go to a queue knowing that if it has a
> buffer in it that it meets are desired size. The geode off-heap free list
> implementation does this and only the largest allocations need to search
> through a free list that has items in it that exceed a max size. Every other
> free list is found quickly be using the requested size to index an array of
> free lists, which only contain items of that size.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
