From b641b5670c06a09536a52c216522aa70abb40460 Mon Sep 17 00:00:00 2001 From: "PSp..om" Date: Wed, 23 Mar 2011 17:45:35 +0000 Subject: [PATCH] Fixed stupid buffer copy bug partially caused by some last minute shifts in APIs. Anyway, outbound TCP broadcasts work again and are more efficient since the Kernel callers can better control when their message buffers are copied (in this case it isn't necessary so we save a buffer copy). git-svn-id: https://jmonkeyengine.googlecode.com/svn/trunk@7088 75d07b2b-3a1a-0410-a2c5-0572b91ccdca --- .../com/jme3/network/base/DefaultServer.java | 6 +++-- .../com/jme3/network/kernel/Kernel.java | 10 +++++-- .../network/kernel/tcp/SelectorKernel.java | 26 ++++++++++++------- .../jme3/network/kernel/udp/UdpKernel.java | 6 ++++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/engine/src/networking/com/jme3/network/base/DefaultServer.java b/engine/src/networking/com/jme3/network/base/DefaultServer.java index 11c3814ea..c71f7aa55 100644 --- a/engine/src/networking/com/jme3/network/base/DefaultServer.java +++ b/engine/src/networking/com/jme3/network/base/DefaultServer.java @@ -160,9 +160,11 @@ public class DefaultServer implements Server // Ignore the filter for the moment if( message.isReliable() || fast == null ) { - reliable.broadcast( adapter, buffer, true ); + // Don't need to copy the data because message protocol is already + // giving us a fresh buffer + reliable.broadcast( adapter, buffer, true, false ); } else { - fast.broadcast( adapter, buffer, false ); + fast.broadcast( adapter, buffer, false, false ); } } diff --git a/engine/src/networking/com/jme3/network/kernel/Kernel.java b/engine/src/networking/com/jme3/network/kernel/Kernel.java index 30e69c754..efb2bfdb7 100644 --- a/engine/src/networking/com/jme3/network/kernel/Kernel.java +++ b/engine/src/networking/com/jme3/network/kernel/Kernel.java @@ -60,9 +60,15 @@ public interface Kernel /** * Dispatches the data to all endpoints managed by the - * kernel that match the specified endpoint filter.. + * kernel that match the specified endpoint filter.. + * If 'copy' is true then the implementation will copy the byte buffer + * before delivering it to endpoints. This allows the caller to reuse + * the data buffer. Though it is important that the buffer not be changed + * by another thread while this call is running. + * Only the bytes from data.position() to data.remaining() are sent. */ - public void broadcast( Filter filter, ByteBuffer data, boolean reliable ); + public void broadcast( Filter filter, ByteBuffer data, boolean reliable, + boolean copy ); /** * Returns true if there are waiting envelopes. diff --git a/engine/src/networking/com/jme3/network/kernel/tcp/SelectorKernel.java b/engine/src/networking/com/jme3/network/kernel/tcp/SelectorKernel.java index c91aef66c..7840f7114 100644 --- a/engine/src/networking/com/jme3/network/kernel/tcp/SelectorKernel.java +++ b/engine/src/networking/com/jme3/network/kernel/tcp/SelectorKernel.java @@ -112,14 +112,19 @@ public class SelectorKernel extends AbstractKernel } } - public void broadcast( Filter filter, ByteBuffer data, boolean reliable ) + public void broadcast( Filter filter, ByteBuffer data, boolean reliable, + boolean copy ) { if( !reliable ) throw new UnsupportedOperationException( "Unreliable send not supported by this kernel." ); - // Copy the data just once - byte[] copy = new byte[data.remaining()]; - System.arraycopy(data.array(), data.position(), copy, 0, data.remaining()); + if( copy ) + { + // Copy the data just once + byte[] temp = new byte[data.remaining()]; + System.arraycopy(data.array(), data.position(), temp, 0, data.remaining()); + data = ByteBuffer.wrap(temp); + } // Hand it to all of the endpoints that match our routing for( NioEndpoint p : endpoints.values() ) { @@ -127,8 +132,10 @@ public class SelectorKernel extends AbstractKernel if( filter != null && !filter.apply(p) ) continue; - // Give it the data - p.send( data, false, false ); + // Give it the data... but let each endpoint track their + // own completion over the shared array of bytes by + // duplicating it + p.send( data.duplicate(), false, false ); } // Wake up the selector so it can reinitialize its @@ -268,9 +275,10 @@ public class SelectorKernel extends AbstractKernel // efficiently done as change requests... or simply // keeping a thread-safe set of endpoints with pending // writes. For most cases, it shouldn't matter. - for( Map.Entry e : endpointKeys.entrySet() ) { - if( e.getKey().hasPending() ) + for( Map.Entry e : endpointKeys.entrySet() ) { + if( e.getKey().hasPending() ) { e.getValue().interestOps(SelectionKey.OP_WRITE); + } } } @@ -363,7 +371,7 @@ public class SelectorKernel extends AbstractKernel } c.write( current ); - + // If we wrote all of that packet then we need to remove it if( current.remaining() == 0 ) { p.removePending(); diff --git a/engine/src/networking/com/jme3/network/kernel/udp/UdpKernel.java b/engine/src/networking/com/jme3/network/kernel/udp/UdpKernel.java index 07ea4019c..904a407f2 100644 --- a/engine/src/networking/com/jme3/network/kernel/udp/UdpKernel.java +++ b/engine/src/networking/com/jme3/network/kernel/udp/UdpKernel.java @@ -114,11 +114,15 @@ public class UdpKernel extends AbstractKernel * Dispatches the data to all endpoints managed by the * kernel. 'routing' is currently ignored. */ - public void broadcast( Filter filter, ByteBuffer data, boolean reliable ) + public void broadcast( Filter filter, ByteBuffer data, boolean reliable, + boolean copy ) { if( reliable ) throw new UnsupportedOperationException( "Reliable send not supported by this kernel." ); + // We ignore the copy flag because we know all outbound traffic + // goes instantly. + // Hand it to all of the endpoints that match our routing for( UdpEndpoint p : socketEndpoints.values() ) { // Does it match the filter?