Thinking about it more, what we really want is for the client to target the per

second rate (10 messages per second), and to do the same 10x buffer on the
server that we were doing before so that we avoid disconnects due to network
congestion (which might "save up" some throttled messages and then end up
delivering them in a bunch with some later messages).


git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@5424 542714f4-19e9-0310-aa3c-eee0fc999fb1

src/java/com/threerings/presents/client/Client.java

@@ -62,8 +62,8 @@ public class Client
     /** The maximum size of a datagram. */
     public static final int MAX_DATAGRAM_SIZE = 1500;
 
-    /** Our default maximum outgoing message rate as { messages, milliseconds }. */
-    public static final int[] DEFAULT_MAX_MSG_RATE = { 50, 5*1000 };
+    /** Our default maximum outgoing message rate in messages per second. */
+    public static final int DEFAULT_MSGS_PER_SECOND = 10;
 
     /**
      * Constructs a client object with the supplied credentials and RunQueue. The creds will be
@@ -82,10 +82,6 @@ public class Client
     {
         _creds = creds;
         _runQueue = runQueue;
-
-        // initialize our default throttle; we use a slightly lowered rate from the default to
-        // avoid edge cases where we and the server disagree about a single millisecond
-        _outThrottle = new Throttle(DEFAULT_MAX_MSG_RATE[0]-1, DEFAULT_MAX_MSG_RATE[1]);
     }
 
     /**
@@ -644,10 +640,10 @@ public class Client
      * Configures the outgoing message throttle. This is done when the server informs us that a new
      * rate is in effect.
      */
-    protected synchronized void setOutgoingMessageThrottle (int msgs, long period)
+    protected synchronized void setOutgoingMessageThrottle (int msgsPerSec)
     {
-        log.info("Updating outgoing message throttle", "msgs", msgs, "period", period);
-        _outThrottle.reinit(msgs, period);
+        log.info("Updating outgoing message throttle", "msgsPerSec", msgsPerSec);
+        _outThrottle.reinit(msgsPerSec, 1000L);
         _comm.postMessage(new ThrottleUpdatedMessage());
     }
 
@@ -1075,7 +1071,7 @@ public class Client
     protected Interval _tickInterval;
 
     /** Our outgoing message throttle. */
-    protected Throttle _outThrottle;
+    protected Throttle _outThrottle = new Throttle(DEFAULT_MSGS_PER_SECOND, 1000L);
 
     /** How often we recompute our time offset from the server. */
     protected static final long CLOCK_SYNC_INTERVAL = 600 * 1000L;

src/java/com/threerings/presents/client/ClientDObjectMgr.java

@@ -202,7 +202,7 @@ public class ClientDObjectMgr
 
             } else if (obj instanceof UpdateThrottleMessage) {
                 UpdateThrottleMessage upmsg = (UpdateThrottleMessage)obj;
-                _client.setOutgoingMessageThrottle(upmsg.messages, upmsg.period);
+                _client.setOutgoingMessageThrottle(upmsg.messagesPerSec);
 
             } else if (obj instanceof ObjectAction<?>) {
                 ObjectAction<?> act = (ObjectAction<?>)obj;

src/java/com/threerings/presents/net/UpdateThrottleMessage.java

@@ -26,24 +26,19 @@ package com.threerings.presents.net;
  */
 public class UpdateThrottleMessage extends DownstreamMessage
 {
-    /** The number of messages allowed per throttle period. */
-    public final int messages;
-
-    /** The throttle period. */
-    public final long period;
+    /** The number of messages allowed per second. */
+    public final int messagesPerSec;
 
     /**
      * Zero argument constructor used when unserializing an instance.
      */
     public UpdateThrottleMessage ()
     {
-        this.messages = 0;
-        this.period = 0;
+        this.messagesPerSec = 0;
     }
 
-    public UpdateThrottleMessage (int messages, long period)
+    public UpdateThrottleMessage (int messagesPerSec)
     {
-        this.messages = messages;
-        this.period = period;
+        this.messagesPerSec = messagesPerSec;
     }
 }

src/java/com/threerings/presents/server/PresentsClient.java

@@ -186,19 +186,12 @@ public class PresentsClient
      * beyond the default for untrusted clients. This mechanism exists so that trusted clients can
      * have their throttle relaxed in a robust manner which will not result in disconnects if the
      * client happens to be at or near the throttle limit when the throttle is reduced.
-     *
-     * @param messages the number of messages allowed in the period.
-     * @param period the throttle period (in milliseconds).
      */
     @EventThread
-    public void setIncomingMessageThrottle (int messages, long period)
+    public void setIncomingMessageThrottle (int messagesPerSec)
     {
-        // we do a couple of things to make sure we don't accidentally hit our throttle: we use 2x
-        // messages/period so that if the client sends all of its messages in 1ms and then tries to
-        // send another full batch in the final ms after the throttle lets up, we don't freak out
-        // if we disagree about that ms and we reduce the client's message count by one
-        _pendingThrottles.add(new int[] { 2*messages, (int)(2*period) });
-        postMessage(new UpdateThrottleMessage(messages-1, period));
+        _pendingThrottles.add(messagesPerSec);
+        postMessage(new UpdateThrottleMessage(messagesPerSec));
         // when we get a ThrottleUpdatedMessage from the client, we'll apply the new throttle
     }
 
@@ -562,7 +555,7 @@ public class PresentsClient
     protected Throttle createIncomingMessageThrottle ()
     {
         // see setIncomingMessageThrottle for more details on all of this
-        return new Throttle(2*Client.DEFAULT_MAX_MSG_RATE[0], 2*Client.DEFAULT_MAX_MSG_RATE[1]);
+        return new Throttle(10*Client.DEFAULT_MSGS_PER_SECOND+1, 10*1000L);
     }
 
     /**
@@ -863,10 +856,14 @@ public class PresentsClient
                                 "client", this);
                     return;
                 }
-                int[] data = _pendingThrottles.remove(0);
-                log.info("Applying updated throttle", "client", this,
-                         "msgs", data[0], "period", data[1]);
-                _throttle.reinit(data[0], data[1]);
+                int messagesPerSec = _pendingThrottles.remove(0);
+                log.info("Applying updated throttle", "client", this, "msgsPerSec", messagesPerSec);
+                // we set our hard throttle over a 10 second period instead of a 1 second period to
+                // account for periods of network congestion that might cause otherwise properly
+                // throttled messages to bunch up while they're "on the wire"; we also add a one
+                // message buffer so that if the client is right up against the limit, we don't end
+                // up quibbling over a couple of milliseconds
+                _throttle.reinit(10*messagesPerSec+1, 10*1000L);
             }
         });
     }
@@ -1096,7 +1093,7 @@ public class PresentsClient
     protected Throttle _throttle = createIncomingMessageThrottle();
 
     /** Used to keep throttles around until we know the client is ready for us to apply them. */
-    protected List<int[]> _pendingThrottles = Lists.newArrayList();
+    protected List<Integer> _pendingThrottles = Lists.newArrayList();
 
     // keep these for kicks and giggles
     protected int _messagesIn;

tests/src/java/com/threerings/presents/client/TestClient.java

@@ -121,7 +121,7 @@ public class TestClient
         object.postMessage("lawl!");
 
         // try blowing through our message limit
-        for (int ii = 0; ii < 2*Client.DEFAULT_MAX_MSG_RATE[0]+5; ii++) {
+        for (int ii = 0; ii < 15*Client.DEFAULT_MSGS_PER_SECOND; ii++) {
             object.postMessage("ZOMG!", new Integer(ii));
         }
     }