RateLimiter.create called with zero warmup fails with real clock

[greggdonovan]

We recently hit an issue where calling RateLimiter.create(16.0, 0, TimeUnit.SECONDS) failed to rate limit after an upgrade from Guava 17 to 21. I tried to reproduce this using the FakeStopwatch but could only get it to reproduce with a real clock.

I'll be happy to submit this as PR if the change below is useful.

Bisecting tells me this changed in this commit.

bisect.sh

#!/usr/bin/env bash

function back() {
  cd ~/development/guava
  git co .
}
trap back EXIT

JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.7.0_80.jdk/Contents/Home

# I can't get guava-gwt to compile :-(
sed -i '' 's_<module>guava-gwt</module>__' pom.xml

mvn clean install -Dmaven.test.skip=true || exit 125
cd guava-tests
{ mvn test -Dtest=com.google.common.util.concurrent.RateLimiterZeroWarmupTest && exit 0; } || exit 127

guava-tests/test/com/google/common/util/concurrent/RateLimiterZeroWarmupTest.java

package com.google.common.util.concurrent;

import java.util.concurrent.TimeUnit;
import junit.framework.TestCase;

public class RateLimiterZeroWarmupTest extends TestCase {

  public void testWarmUpWithZeroWarmupAndRealClock() {
    RateLimiter limiter = RateLimiter.create(16.0, 0, TimeUnit.SECONDS);
    int totalPermits = 1000;
    double totalSleep = 0;
    for (int i = 0; i < totalPermits; i++) {
      totalSleep += limiter.acquire();
    }
    assertFalse("totalSleep should be more than 60s, not 0", totalSleep == 0.0d); // <-- this fails
  }
}

Using the FakeStopwatch fails to reproduce:

diff --git a/guava-tests/test/com/google/common/util/concurrent/RateLimiterTest.java b/guava-tests/test/com/google/common/util/concurrent/RateLimiterTest.java
index b143bd94a..0c4de6c86 100644
--- a/guava-tests/test/com/google/common/util/concurrent/RateLimiterTest.java
+++ b/guava-tests/test/com/google/common/util/concurrent/RateLimiterTest.java
@@ -312,6 +312,14 @@ public class RateLimiterTest extends TestCase {
         "R0.20, R0.10, R0.10, R0.10"); // #7 (cont.), note, this matches #5
   }
 
+  public void testWarmUpWithZeroWarmup() {
+    RateLimiter rateLimiter = RateLimiter.create(stopwatch, 5.0, 0, TimeUnit.SECONDS, 3.0);
+    for (int i = 0; i < 5; i++) {
+      rateLimiter.acquire(); // #1
+      }
+      assertEvents("R0.00, R0.20, R0.20, R0.20, R0.20" /* #1 */); // <-- this works
+  }
+
   public void testBurstyAndUpdate() {
     RateLimiter rateLimiter = RateLimiter.create(stopwatch, 1.0);
     rateLimiter.acquire(1); // no wait

Guava workaround to use SmoothBursty when warmupSeconds == 0:

diff --git a/guava/src/com/google/common/util/concurrent/RateLimiter.java b/guava/src/com/google/common/util/concurrent/RateLimiter.java
index 945e2d168..cc99bbcbe 100644
--- a/guava/src/com/google/common/util/concurrent/RateLimiter.java
+++ b/guava/src/com/google/common/util/concurrent/RateLimiter.java
@@ -159,6 +159,9 @@ public abstract class RateLimiter {
    */
   public static RateLimiter create(double permitsPerSecond, long warmupPeriod, TimeUnit unit) {
     checkArgument(warmupPeriod >= 0, "warmupPeriod must not be negative: %s", warmupPeriod);
+    if (warmupPeriod == 0) {
+      return create(permitsPerSecond);
+    }
     return create(
         SleepingStopwatch.createFromSystemTimer(), permitsPerSecond, warmupPeriod, unit, 3.0);
   }