From 9b4dacb8d13f6be9ca49b702a1371c3e62d616d5 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@vmware.com>
Date: Tue, 17 May 2022 11:18:07 +0200
Subject: [PATCH] Improve codec caching.

Codecs are now cached using their value type for simple value/null encoding. Additionally, for decoding we check whether the cached codec can decode the desired format/data type/value type and fall back to the cache supplier if there was a hash collision.

[resolves #511]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
---
 .../postgresql/codec/CachedCodecLookup.java   | 50 ++++++++++---------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/src/main/java/io/r2dbc/postgresql/codec/CachedCodecLookup.java b/src/main/java/io/r2dbc/postgresql/codec/CachedCodecLookup.java
index 5f69b8d52..cc1abc2af 100644
--- a/src/main/java/io/r2dbc/postgresql/codec/CachedCodecLookup.java
+++ b/src/main/java/io/r2dbc/postgresql/codec/CachedCodecLookup.java
@@ -40,9 +40,9 @@ class CachedCodecLookup implements CodecLookup {
 
     private final Map<Integer, Codec<?>> decodeCodecsCache = new ConcurrentHashMap<>();
 
-    private final Map<Integer, Codec<?>> encodeCodecsCache = new ConcurrentHashMap<>();
+    private final Map<Class<?>, Codec<?>> encodeCodecsCache = new ConcurrentHashMap<>();
 
-    private final Map<Integer, Codec<?>> encodeNullCodecsCache = new ConcurrentHashMap<>();
+    private final Map<Class<?>, Codec<?>> encodeNullCodecsCache = new ConcurrentHashMap<>();
 
     private final CodecLookup delegate;
 
@@ -100,7 +100,7 @@ public void afterCodecAdded() {
     @Override
     public <T> Codec<T> findDecodeCodec(int dataType, Format format, Class<? extends T> type) {
         Integer hash = generateCodecHash(dataType, format, type);
-        return findCodec(hash, this.decodeCodecsCache, () -> {
+        return findCodec(hash, dataType, format, type, this.decodeCodecsCache, () -> {
             LOG.trace("[codec-finder dataType={}, format={}, type={}] Decode codec not found in cache", dataType, format, type.getName());
             Codec<T> c = this.delegate.findDecodeCodec(dataType, format, type);
             if (c != null) {
@@ -112,12 +112,11 @@ public <T> Codec<T> findDecodeCodec(int dataType, Format format, Class<? extends
 
     @Override
     public <T> Codec<T> findEncodeCodec(T value) {
-        Integer hash = generateCodecHash(value.getClass());
-        return findCodec(hash, this.encodeCodecsCache, () -> {
+        return findCodec(value.getClass(), this.encodeCodecsCache, () -> {
             LOG.trace("[codec-finder type={}] Encode codec not found in cache", value.getClass().getName());
             Codec<T> c = this.delegate.findEncodeCodec(value);
             if (c != null) {
-                this.encodeCodecsCache.putIfAbsent(hash, c);
+                this.encodeCodecsCache.putIfAbsent(value.getClass(), c);
             }
             return c;
         });
@@ -125,12 +124,11 @@ public <T> Codec<T> findEncodeCodec(T value) {
 
     @Override
     public <T> Codec<T> findEncodeNullCodec(Class<T> type) {
-        Integer hash = generateCodecHash(type);
-        return findCodec(hash, this.encodeNullCodecsCache, () -> {
+        return findCodec(type, this.encodeNullCodecsCache, () -> {
             LOG.trace("[codec-finder type={}] Encode null codec not found in cache", type.getName());
             Codec<T> c = this.delegate.findEncodeNullCodec(type);
             if (c != null) {
-                this.encodeNullCodecsCache.putIfAbsent(hash, c);
+                this.encodeNullCodecsCache.putIfAbsent(type, c);
             }
             return c;
         });
@@ -142,32 +140,36 @@ private void cacheDecode(Codec<?> c, Class<?> type, PostgresTypeIdentifier ident
     }
 
     private void cacheEncode(Codec<?> c, Class<?> type) {
-        Integer encodeHash = generateCodecHash(type);
-        this.encodeCodecsCache.putIfAbsent(encodeHash, c);
+        this.encodeCodecsCache.putIfAbsent(type, c);
         if (c.canEncodeNull(type)) {
-            this.encodeNullCodecsCache.putIfAbsent(encodeHash, c);
+            this.encodeNullCodecsCache.putIfAbsent(type, c);
         }
     }
 
     @SuppressWarnings("unchecked")
-    private synchronized <T> Codec<T> findCodec(Integer codecHash, Map<Integer, Codec<?>> cache, Supplier<Codec<T>> fallback) {
-        Codec<T> value = (Codec<T>) cache.get(codecHash);
+    private synchronized <T> Codec<T> findCodec(Class<?> cacheKey, Map<Class<?>, Codec<?>> cache, Supplier<Codec<T>> fallback) {
+        Codec<T> value = (Codec<T>) cache.get(cacheKey);
         return value != null ? value : fallback.get();
     }
 
-    private static Integer generateCodecHash(int dataType, Format format, Class<?> type) {
-        int hash = (dataType << 5) - dataType;
-        hash = (hash << 5) - hash + format.hashCode();
-        hash = (hash << 5) - hash + generateCodecHash(type);
-        return hash;
+    @SuppressWarnings("unchecked")
+    private synchronized <T> Codec<T> findCodec(Integer cacheKey, int dataType, Format format, Class<? extends T> type, Map<Integer, Codec<?>> cache, Supplier<Codec<T>> fallback) {
+        Codec<T> value = (Codec<T>) cache.get(cacheKey);
+        return (value != null && value.canDecode(dataType, format, type)) ? value : fallback.get();
     }
 
-    private static Integer generateCodecHash(Class<?> type) {
-        int hash = type.hashCode();
-        if (type.getComponentType() != null) {
-            hash = (hash << 5) - hash + generateCodecHash(type.getComponentType());
+    private static Integer generateCodecHash(int dataType, Format format, Class<?> type) {
+        int result = 1;
+
+        result = 31 * result + dataType;
+        result = 31 * result + format.hashCode();
+        result = 31 * result + type.hashCode();
+
+        if (type.isArray()) {
+            result = 31 * result + type.getComponentType().hashCode();
         }
-        return hash;
+
+        return result;
     }
 
 }