From dcfb27288ac9bce711b92bf9f7f3500a78a68913 Mon Sep 17 00:00:00 2001 From: John Hurliman Date: Thu, 12 Aug 2010 19:20:37 +0000 Subject: [PATCH] * Fix packet sending to only send ACKs for incoming packets marked as reliable, not every incoming packet * Fixed a bug in ACK sending that was eating up CPU time * Put the final UDP packets on the wire synchronously instead of asynchronously. Profiling showed this to be faster than .NET 2.0 style asynchronous sending (although possibly still slower than .NET 3.5 style asynchronous sending) * Small tweak to LocklessQueue to try and keep LocklessQueue.Count as accurate as possible git-svn-id: http://libopenmetaverse.googlecode.com/svn/libopenmetaverse/trunk@3417 52acb1d6-8a22-11de-b505-999d5b087335 --- OpenMetaverse/Simulator.cs | 60 +++++++++++++++-------------- OpenMetaverse/UDPBase.cs | 41 ++++++++++++-------- OpenMetaverseTypes/LocklessQueue.cs | 1 + 3 files changed, 57 insertions(+), 45 deletions(-) diff --git a/OpenMetaverse/Simulator.cs b/OpenMetaverse/Simulator.cs index 0fb7cae0..9f80e9ce 100644 --- a/OpenMetaverse/Simulator.cs +++ b/OpenMetaverse/Simulator.cs @@ -450,7 +450,6 @@ namespace OpenMetaverse private Queue InBytes, OutBytes; // ACKs that are queued up to be sent to the simulator private LocklessQueue PendingAcks = new LocklessQueue(); - private int PendingAckCount = 0; private Timer AckTimer; private Timer PingTimer; private Timer StatsTimer; @@ -838,7 +837,7 @@ namespace OpenMetaverse // Set the last byte of the packet equal to the number of appended ACKs buffer.Data[dataLength++] = (byte)ackCount; // Set the appended ACKs flag on this packet - buffer.Data[0] = (byte)(buffer.Data[0] | Helpers.MSG_APPENDED_ACKS); + buffer.Data[0] |= Helpers.MSG_APPENDED_ACKS; } buffer.DataLength = dataLength; @@ -1022,30 +1021,32 @@ namespace OpenMetaverse #endregion ACK Receiving - #region ACK Sending - - // Add this packet to the list of ACKs that need to be sent out - uint sequence = (uint)packet.Header.Sequence; - PendingAcks.Enqueue(sequence); - int pendingAckCount = Interlocked.Increment(ref PendingAckCount); - - // Send out ACKs if we have a lot of them - if (pendingAckCount >= Client.Settings.MAX_PENDING_ACKS) - SendAcks(); - - #endregion ACK Sending - - // Check the archive of received packet IDs to see whether we already received this packet - if (packet.Header.Reliable && !PacketArchive.TryEnqueue(packet.Header.Sequence)) + if (packet.Header.Reliable) { - if (packet.Header.Resent) - Logger.DebugLog("Received a resend of already processed packet #" + packet.Header.Sequence + ", type: " + packet.Type); - else - Logger.Log("Received a duplicate (not marked as resend) of packet #" + packet.Header.Sequence + ", type: " + packet.Type, - Helpers.LogLevel.Warning); + #region ACK Sending - // Avoid firing a callback twice for the same packet - return; + // Add this packet to the list of ACKs that need to be sent out + uint sequence = (uint)packet.Header.Sequence; + PendingAcks.Enqueue(sequence); + + // Send out ACKs if we have a lot of them + if (PendingAcks.Count >= Client.Settings.MAX_PENDING_ACKS) + SendAcks(); + + #endregion ACK Sending + + // Check the archive of received packet IDs to see whether we already received this packet + if (!PacketArchive.TryEnqueue(packet.Header.Sequence)) + { + if (packet.Header.Resent) + Logger.DebugLog("Received a resend of already processed packet #" + packet.Header.Sequence + ", type: " + packet.Type); + else + Logger.Log("Received a duplicate (not marked as resend) of packet #" + packet.Header.Sequence + ", type: " + packet.Type, + Helpers.LogLevel.Warning); + + // Avoid firing a callback twice for the same packet + return; + } } #region Inbox Insertion @@ -1079,14 +1080,14 @@ namespace OpenMetaverse /// /// Sends out pending acknowledgements /// - private void SendAcks() + /// Number of ACKs sent + private int SendAcks() { uint ack; + int ackCount = 0; if (PendingAcks.TryDequeue(out ack)) { - Interlocked.Decrement(ref PendingAckCount); - List blocks = new List(); PacketAckPacket.PacketsBlock block = new PacketAckPacket.PacketsBlock(); block.ID = ack; @@ -1094,8 +1095,6 @@ namespace OpenMetaverse while (PendingAcks.TryDequeue(out ack)) { - Interlocked.Decrement(ref PendingAckCount); - block = new PacketAckPacket.PacketsBlock(); block.ID = ack; blocks.Add(block); @@ -1105,8 +1104,11 @@ namespace OpenMetaverse packet.Header.Reliable = false; packet.Packets = blocks.ToArray(); + ackCount = blocks.Count; SendPacket(packet); } + + return ackCount; } /// diff --git a/OpenMetaverse/UDPBase.cs b/OpenMetaverse/UDPBase.cs index f40a647e..efade0cf 100644 --- a/OpenMetaverse/UDPBase.cs +++ b/OpenMetaverse/UDPBase.cs @@ -225,32 +225,41 @@ namespace OpenMetaverse { try { - udpSocket.BeginSendTo( + // Profiling heavily loaded clients was showing better performance with + // synchronous UDP packet sending + udpSocket.SendTo( buf.Data, 0, buf.DataLength, SocketFlags.None, - buf.RemoteEndPoint, - AsyncEndSend, - buf); + buf.RemoteEndPoint); + + //udpSocket.BeginSendTo( + // buf.Data, + // 0, + // buf.DataLength, + // SocketFlags.None, + // buf.RemoteEndPoint, + // AsyncEndSend, + // buf); } catch (SocketException) { } catch (ObjectDisposedException) { } } } - void AsyncEndSend(IAsyncResult result) - { - try - { - UDPPacketBuffer buf = (UDPPacketBuffer)result.AsyncState; - if (!udpSocket.Connected) return; - int bytesSent = udpSocket.EndSendTo(result); + //void AsyncEndSend(IAsyncResult result) + //{ + // try + // { + // UDPPacketBuffer buf = (UDPPacketBuffer)result.AsyncState; + // if (!udpSocket.Connected) return; + // int bytesSent = udpSocket.EndSendTo(result); - PacketSent(buf, bytesSent); - } - catch (SocketException) { } - catch (ObjectDisposedException) { } - } + // PacketSent(buf, bytesSent); + // } + // catch (SocketException) { } + // catch (ObjectDisposedException) { } + //} } } diff --git a/OpenMetaverseTypes/LocklessQueue.cs b/OpenMetaverseTypes/LocklessQueue.cs index 85c9f803..0dad1087 100644 --- a/OpenMetaverseTypes/LocklessQueue.cs +++ b/OpenMetaverseTypes/LocklessQueue.cs @@ -126,6 +126,7 @@ namespace OpenMetaverse if (oldHeadNext == null) { item = default(T); + count = 0; return false; } if (CAS(ref head, oldHead, oldHeadNext))