Cookiejar optimization

client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

@@ -198,6 +198,13 @@ public interface AsyncHttpClientConfig {
    */
   CookieStore getCookieStore();
 
+  /**
+   * Return the delay in milliseconds to evict expired cookies from {@linkplain CookieStore}
+   *
+   * @return the delay in milliseconds to evict expired cookies from {@linkplain CookieStore}
+   */
+  int expiredCookieEvictionDelay();
+
   /**
    * Return the number of time the library will retry when an {@link java.io.IOException} is throw by the remote server
    *

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java

@@ -22,6 +22,7 @@
 import io.netty.util.Timer;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import org.asynchttpclient.channel.ChannelPool;
+import org.asynchttpclient.cookie.CookieEvictionTask;
 import org.asynchttpclient.filter.FilterContext;
 import org.asynchttpclient.filter.FilterException;
 import org.asynchttpclient.filter.RequestFilter;
@@ -90,6 +91,8 @@ public DefaultAsyncHttpClient(AsyncHttpClientConfig config) {
     channelManager = new ChannelManager(config, nettyTimer);
     requestSender = new NettyRequestSender(config, channelManager, nettyTimer, new AsyncHttpClientState(closed));
     channelManager.configureBootstraps(requestSender);
+    nettyTimer.newTimeout(new CookieEvictionTask(config.expiredCookieEvictionDelay(), config.getCookieStore()),
+                          config.expiredCookieEvictionDelay(), TimeUnit.MILLISECONDS);
   }
 
   private Timer newNettyTimer(AsyncHttpClientConfig config) {

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

@@ -109,6 +109,7 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig {
 
   // cookie store
   private final CookieStore cookieStore;
+  private final int expiredCookieEvictionDelay;
 
   // internals
   private final String threadPoolName;
@@ -192,6 +193,7 @@ private DefaultAsyncHttpClientConfig(// http
 
                                        // cookie store
                                        CookieStore cookieStore,
+                                       int expiredCookieEvictionDelay,
 
                                        // tuning
                                        boolean tcpNoDelay,
@@ -283,6 +285,7 @@ private DefaultAsyncHttpClientConfig(// http
 
     // cookie store
     this.cookieStore = cookieStore;
+    this.expiredCookieEvictionDelay = expiredCookieEvictionDelay;
 
     // tuning
     this.tcpNoDelay = tcpNoDelay;
@@ -558,6 +561,11 @@ public CookieStore getCookieStore() {
     return cookieStore;
   }
 
+  @Override
+  public int expiredCookieEvictionDelay() {
+    return expiredCookieEvictionDelay;
+  }
+
   // tuning
   @Override
   public boolean isTcpNoDelay() {
@@ -746,6 +754,7 @@ public static class Builder {
 
     // cookie store
     private CookieStore cookieStore = new ThreadSafeCookieStore();
+    private int expiredCookieEvictionDelay = defaultExpiredCookieEvictionDelay();
 
     // tuning
     private boolean tcpNoDelay = defaultTcpNoDelay();
@@ -1146,6 +1155,11 @@ public Builder setCookieStore(CookieStore cookieStore) {
       return this;
     }
 
+    public Builder setExpiredCookieEvictionDelay(int expiredCookieEvictionDelay) {
+      this.expiredCookieEvictionDelay = expiredCookieEvictionDelay;
+      return this;
+    }
+
     // tuning
     public Builder setTcpNoDelay(boolean tcpNoDelay) {
       this.tcpNoDelay = tcpNoDelay;
@@ -1330,6 +1344,7 @@ public DefaultAsyncHttpClientConfig build() {
               responseFilters.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(responseFilters),
               ioExceptionFilters.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(ioExceptionFilters),
               cookieStore,
+              expiredCookieEvictionDelay,
               tcpNoDelay,
               soReuseAddress,
               soKeepAlive,

client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java

@@ -73,6 +73,7 @@ public final class AsyncHttpClientConfigDefaults {
   public static final String IO_THREADS_COUNT_CONFIG = "ioThreadsCount";
   public static final String HASHED_WHEEL_TIMER_TICK_DURATION = "hashedWheelTimerTickDuration";
   public static final String HASHED_WHEEL_TIMER_SIZE = "hashedWheelTimerSize";
+  public static final String EXPIRED_COOKIE_EVICTION_DELAY = "expiredCookieEvictionDelay";
 
   public static final String AHC_VERSION;
 
@@ -304,4 +305,8 @@ public static int defaultHashedWheelTimerTickDuration() {
   public static int defaultHashedWheelTimerSize() {
     return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getInt(ASYNC_CLIENT_CONFIG_ROOT + HASHED_WHEEL_TIMER_SIZE);
   }
+
+  public static int defaultExpiredCookieEvictionDelay() {
+    return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getInt(ASYNC_CLIENT_CONFIG_ROOT + EXPIRED_COOKIE_EVICTION_DELAY);
+  }
 }

client/src/main/java/org/asynchttpclient/cookie/CookieEvictionTask.java

@@ -0,0 +1,30 @@
+package org.asynchttpclient.cookie;
+
+import java.util.concurrent.TimeUnit;
+
+import org.asynchttpclient.AsyncHttpClientConfig;
+
+import io.netty.util.Timeout;
+import io.netty.util.TimerTask;
+
+/**
+ * Evicts expired cookies from the {@linkplain CookieStore} periodically.
+ * The default delay is 30 seconds. Ypu may override the default using
+ * {@linkplain AsyncHttpClientConfig#expiredCookieEvictionDelay()}.
+ */
+public class CookieEvictionTask implements TimerTask {
+
+    private long evictDelayInMs;
+    private CookieStore cookieStore;
+
+    public CookieEvictionTask(long evictDelayInMs, CookieStore cookieStore) {
+        this.evictDelayInMs = evictDelayInMs;
+        this.cookieStore = cookieStore;
+    }
+
+    @Override
+    public void run(Timeout timeout) throws Exception {
+        cookieStore.evictExpired();
+        timeout.timer().newTimeout(this, evictDelayInMs, TimeUnit.MILLISECONDS);
+    }
+}

client/src/main/java/org/asynchttpclient/cookie/CookieStore.java

@@ -34,7 +34,7 @@
 public interface CookieStore {
   /**
    * Adds one {@link Cookie} to the store. This is called for every incoming HTTP response.
-   * If the given cookie has already expired it will not be added, but existing values will still be removed.
+   * If the given cookie has already expired it will not be added.
    *
    * <p>A cookie to store may or may not be associated with an URI. If it
    * is not associated with an URI, the cookie's domain and path attribute
@@ -82,4 +82,9 @@ public interface CookieStore {
    * @return true if any cookies were purged.
    */
   boolean clear();
+
+  /**
+   * Evicts all the cookies that expired as of the time this method is run.
+   */
+  void evictExpired();
 }

client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java

@@ -26,7 +26,7 @@
 
 public final class ThreadSafeCookieStore implements CookieStore {
 
-  private Map<CookieKey, StoredCookie> cookieJar = new ConcurrentHashMap<>();
+  private final Map<String, Map<CookieKey, StoredCookie>> cookieJar = new ConcurrentHashMap<>();
 
   @Override
   public void add(Uri uri, Cookie cookie) {
@@ -43,28 +43,29 @@ public List<Cookie> get(Uri uri) {
 
   @Override
   public List<Cookie> getAll() {
-    final boolean[] removeExpired = {false};
     List<Cookie> result = cookieJar
-            .entrySet()
+            .values()
             .stream()
-            .filter(pair -> {
-              boolean hasCookieExpired = hasCookieExpired(pair.getValue().cookie, pair.getValue().createdAt);
-              if (hasCookieExpired && !removeExpired[0])
-                removeExpired[0] = true;
-              return !hasCookieExpired;
-            })
-            .map(pair -> pair.getValue().cookie)
+            .flatMap(map -> map.values().stream())
+            .filter(pair -> !hasCookieExpired(pair.cookie, pair.createdAt))
+            .map(pair -> pair.cookie)
             .collect(Collectors.toList());
 
-    if (removeExpired[0])
-      removeExpired();
-
     return result;
   }
 
   @Override
   public boolean remove(Predicate<Cookie> predicate) {
-    return cookieJar.entrySet().removeIf(v -> predicate.test(v.getValue().cookie));
+    final boolean[] removed = {false};
+    cookieJar.forEach((key, value) -> {
+      if (!removed[0]) {
+        removed[0] = value.entrySet().removeIf(v -> predicate.test(v.getValue().cookie));
+      }
+    });
+    if (removed[0]) {
+      cookieJar.entrySet().removeIf(entry -> entry.getValue() == null || entry.getValue().isEmpty());
+    }
+    return removed[0];
   }
 
   @Override
@@ -74,8 +75,18 @@ public boolean clear() {
     return result;
   }
 
+  @Override
+  public void evictExpired() {
+    removeExpired();
+  }
+
   ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
+  // Visible for test
+  public Map<String, Map<CookieKey, StoredCookie>> getUnderlying() {
+    return new HashMap<>(cookieJar);
+  }
+
   private String requestDomain(Uri requestUri) {
     return requestUri.getHost().toLowerCase();
   }
@@ -126,13 +137,6 @@ private boolean hasCookieExpired(Cookie cookie, long whenCreated) {
       return false;
   }
 
-  // rfc6265#section-5.1.3
-  // check "The string is a host name (i.e., not an IP address)" ignored
-  private boolean domainsMatch(String cookieDomain, String requestDomain, boolean hostOnly) {
-    return (hostOnly && Objects.equals(requestDomain, cookieDomain)) ||
-            (Objects.equals(requestDomain, cookieDomain) || requestDomain.endsWith("." + cookieDomain));
-  }
-
   // rfc6265#section-5.1.4
   private boolean pathsMatch(String cookiePath, String requestPath) {
     return Objects.equals(cookiePath, requestPath) ||
@@ -145,45 +149,64 @@ private void add(String requestDomain, String requestPath, Cookie cookie) {
     String keyDomain = pair.getKey();
     boolean hostOnly = pair.getValue();
     String keyPath = cookiePath(cookie.path(), requestPath);
-    CookieKey key = new CookieKey(cookie.name().toLowerCase(), keyDomain, keyPath);
+    CookieKey key = new CookieKey(cookie.name().toLowerCase(), keyPath);
 
     if (hasCookieExpired(cookie, 0))
-      cookieJar.remove(key);
-    else
-      cookieJar.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE));
+      cookieJar.getOrDefault(keyDomain, Collections.emptyMap()).remove(key);
+    else {
+      cookieJar.putIfAbsent(keyDomain, new ConcurrentHashMap<>());
+      cookieJar.get(keyDomain).put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE));
+    }
   }
 
   private List<Cookie> get(String domain, String path, boolean secure) {
 
     final boolean[] removeExpired = {false};
+    final List<Cookie> results = new ArrayList<>();
+    boolean exactDomainMatch = true;
+    String subDomain = domain;
+
+    while (MiscUtils.isNonEmpty(subDomain)) {
+      results.addAll(getStoredCookies(subDomain, path, secure, removeExpired, exactDomainMatch));
+      exactDomainMatch = false;
+      subDomain = DomainUtils.getSubDomain(subDomain);
+    }
+
+    return results;
+  }
 
-    List<Cookie> result = cookieJar.entrySet().stream().filter(pair -> {
+  private List<Cookie> getStoredCookies(String domain, String path, boolean secure,
+                                        boolean[] removeExpired, boolean isExactMatch) {
+    if (!cookieJar.containsKey(domain)) {
+      return Collections.emptyList();
+    }
+
+    return cookieJar.get(domain).entrySet().stream().filter(pair -> {
       CookieKey key = pair.getKey();
       StoredCookie storedCookie = pair.getValue();
       boolean hasCookieExpired = hasCookieExpired(storedCookie.cookie, storedCookie.createdAt);
-      if (hasCookieExpired && !removeExpired[0])
-        removeExpired[0] = true;
-      return !hasCookieExpired && domainsMatch(key.domain, domain, storedCookie.hostOnly) && pathsMatch(key.path, path) && (secure || !storedCookie.cookie.isSecure());
+      return !hasCookieExpired &&
+             (isExactMatch || !storedCookie.hostOnly) &&
+             pathsMatch(key.path, path) &&
+             (secure || !storedCookie.cookie.isSecure());
     }).map(v -> v.getValue().cookie).collect(Collectors.toList());
-
-    if (removeExpired[0])
-      removeExpired();
-
-    return result;
   }
 
   private void removeExpired() {
-    cookieJar.entrySet().removeIf(v -> hasCookieExpired(v.getValue().cookie, v.getValue().createdAt));
+    final boolean[] removed = {false};
+    cookieJar.values().forEach(cookieMap -> removed[0] |= cookieMap.entrySet().removeIf(
+            v -> hasCookieExpired(v.getValue().cookie, v.getValue().createdAt)));
+    if (removed[0]) {
+      cookieJar.entrySet().removeIf(entry -> entry.getValue() == null || entry.getValue().isEmpty());
+    }
   }
 
   private static class CookieKey implements Comparable<CookieKey> {
     final String name;
-    final String domain;
     final String path;
 
-    CookieKey(String name, String domain, String path) {
+    CookieKey(String name, String path) {
       this.name = name;
-      this.domain = domain;
       this.path = path;
     }
 
@@ -192,7 +215,6 @@ public int compareTo(CookieKey o) {
       Assertions.assertNotNull(o, "Parameter can't be null");
       int result;
       if ((result = this.name.compareTo(o.name)) == 0)
-        if ((result = this.domain.compareTo(o.domain)) == 0)
           result = this.path.compareTo(o.path);
 
       return result;
@@ -207,14 +229,13 @@ public boolean equals(Object obj) {
     public int hashCode() {
       int result = 17;
       result = 31 * result + name.hashCode();
-      result = 31 * result + domain.hashCode();
       result = 31 * result + path.hashCode();
       return result;
     }
 
     @Override
     public String toString() {
-      return String.format("%s: %s; %s", name, domain, path);
+      return String.format("%s: %s", name, path);
     }
   }
 
@@ -235,4 +256,20 @@ public String toString() {
       return String.format("%s; hostOnly %s; persistent %s", cookie.toString(), hostOnly, persistent);
     }
   }
+
+  public static final class DomainUtils {
+    private static final char DOT = '.';
+      public static String getSubDomain(String domain) {
+        if (domain == null || domain.isEmpty()) {
+          return null;
+        }
+        final int indexOfDot = domain.indexOf(DOT);
+        if (indexOfDot == -1) {
+          return null;
+        }
+        return domain.substring(indexOfDot + 1);
+      }
+
+    private DomainUtils() {}
+  }
 }

client/src/main/resources/org/asynchttpclient/config/ahc-default.properties

@@ -52,3 +52,4 @@ org.asynchttpclient.useNativeTransport=false
 org.asynchttpclient.ioThreadsCount=0
 org.asynchttpclient.hashedWheelTimerTickDuration=100
 org.asynchttpclient.hashedWheelTimerSize=512
+org.asynchttpclient.expiredCookieEvictionDelay=30000