Quick proof-of-concept that strips unnecessary deserialization calls

clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/loaderwriter/DelegatingLoaderWriterStore.java

@@ -106,12 +106,12 @@ public Iterator<Cache.Entry<K, ValueHolder<V>>> iterator() {
   }
 
   @Override
-  public ValueHolder<V> getAndCompute(K key, BiFunction<? super K, ? super V, ? extends V> mappingFunction) throws StoreAccessException {
+  public ValueHolder<V> getAndCompute(K key, BiFunction<? super K, ? super ValueHolder<V>, ? extends V> mappingFunction) throws StoreAccessException {
     throw new UnsupportedOperationException("Implement me");
   }
 
   @Override
-  public ValueHolder<V> computeAndGet(K key, BiFunction<? super K, ? super V, ? extends V> mappingFunction, Supplier<Boolean> replaceEqual, Supplier<Boolean> invokeWriter) throws StoreAccessException {
+  public ValueHolder<V> computeAndGet(K key, BiFunction<? super K, ? super ValueHolder<V>, ? extends V> mappingFunction, Supplier<Boolean> replaceEqual, Supplier<Boolean> invokeWriter) throws StoreAccessException {
     throw new UnsupportedOperationException("Implement me");
   }
 
@@ -121,12 +121,12 @@ public ValueHolder<V> computeIfAbsent(K key, Function<? super K, ? extends V> ma
   }
 
   @Override
-  public Map<K, ValueHolder<V>> bulkCompute(Set<? extends K> keys, Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction) throws StoreAccessException {
+  public Map<K, ValueHolder<V>> bulkCompute(Set<? extends K> keys, Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction) throws StoreAccessException {
     return delegate.bulkCompute(keys, remappingFunction);
   }
 
   @Override
-  public Map<K, ValueHolder<V>> bulkCompute(Set<? extends K> keys, Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction, Supplier<Boolean> replaceEqual) throws StoreAccessException {
+  public Map<K, ValueHolder<V>> bulkCompute(Set<? extends K> keys, Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction, Supplier<Boolean> replaceEqual) throws StoreAccessException {
     throw new UnsupportedOperationException("Implement me");
   }
 

clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/store/ClusteredStore.java

@@ -49,6 +49,7 @@
 import org.ehcache.core.spi.service.ExecutionService;
 import org.ehcache.core.spi.service.StatisticsService;
 import org.ehcache.core.spi.store.Store;
+import org.ehcache.core.spi.store.Store.ValueHolder;
 import org.ehcache.core.spi.store.events.StoreEventFilter;
 import org.ehcache.core.spi.store.events.StoreEventListener;
 import org.ehcache.core.spi.store.events.StoreEventSource;
@@ -478,13 +479,13 @@ public Cache.Entry<K, ValueHolder<V>> next() throws StoreAccessException {
   }
 
   @Override
-  public ValueHolder<V> getAndCompute(final K key, final BiFunction<? super K, ? super V, ? extends V> mappingFunction) {
+  public ValueHolder<V> getAndCompute(final K key, final BiFunction<? super K, ? super ValueHolder<V>, ? extends V> mappingFunction) {
     // TODO: Make appropriate ServerStoreProxy call
     throw new UnsupportedOperationException("Implement me");
   }
 
   @Override
-  public ValueHolder<V> computeAndGet(final K key, final BiFunction<? super K, ? super V, ? extends V> mappingFunction, final Supplier<Boolean> replaceEqual, Supplier<Boolean> invokeWriter) {
+  public ValueHolder<V> computeAndGet(final K key, final BiFunction<? super K, ? super ValueHolder<V>, ? extends V> mappingFunction, final Supplier<Boolean> replaceEqual, Supplier<Boolean> invokeWriter) {
     // TODO: Make appropriate ServerStoreProxy call
     throw new UnsupportedOperationException("Implement me");
   }
@@ -499,7 +500,7 @@ public ValueHolder<V> computeIfAbsent(final K key, final Function<? super K, ? e
    * The assumption is that this method will be invoked only by cache.putAll and cache.removeAll methods.
    */
   @Override
-  public Map<K, ValueHolder<V>> bulkCompute(final Set<? extends K> keys, final Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction)
+  public Map<K, ValueHolder<V>> bulkCompute(final Set<? extends K> keys, final Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction)
       throws StoreAccessException {
     Map<K, ValueHolder<V>> valueHolderMap = new HashMap<>();
     if(remappingFunction instanceof Ehcache.PutAllFunction) {
@@ -525,7 +526,7 @@ public Map<K, ValueHolder<V>> bulkCompute(final Set<? extends K> keys, final Fun
   }
 
   @Override
-  public Map<K, ValueHolder<V>> bulkCompute(final Set<? extends K> keys, final Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction, final Supplier<Boolean> replaceEqual) {
+  public Map<K, ValueHolder<V>> bulkCompute(final Set<? extends K> keys, final Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> remappingFunction, final Supplier<Boolean> replaceEqual) {
     // TODO: Make appropriate ServerStoreProxy call
     throw new UnsupportedOperationException("Implement me");
   }

clustered/ehcache-client/src/test/java/org/ehcache/clustered/client/internal/store/ClusteredStoreTest.java

@@ -520,7 +520,7 @@ public void testBulkComputeRemoveAll() throws Exception {
   @Test(expected = UnsupportedOperationException.class)
   public void testBulkComputeThrowsForGenericFunction() throws Exception {
     @SuppressWarnings("unchecked")
-    Function<Iterable<? extends Map.Entry<? extends Long, ? extends String>>, Iterable<? extends Map.Entry<? extends Long, ? extends String>>> remappingFunction
+    Function<Iterable<? extends Map.Entry<? extends Long, ? extends Store.ValueHolder<String>>>, Iterable<? extends Map.Entry<? extends Long, ? extends String>>> remappingFunction
         = mock(Function.class);
     store.bulkCompute(new HashSet<>(Arrays.asList(1L, 2L)), remappingFunction);
   }

clustered/integration-test/src/test/java/org/ehcache/clustered/JCacheClusteredTest.java

@@ -47,6 +47,7 @@ public class JCacheClusteredTest {
 
   private static final Properties TCK_PROPERTIES = new Properties();
   static {
+    TCK_PROPERTIES.setProperty("org.jsr107.tck.support.server.address", "127.0.0.1");
     TCK_PROPERTIES.setProperty("java.net.preferIPv4Stack", "true");
     TCK_PROPERTIES.setProperty("javax.management.builder.initial", "org.ehcache.jsr107.internal.tck.Eh107MBeanServerBuilder");
     TCK_PROPERTIES.setProperty("org.jsr107.tck.management.agentId", "Eh107MBeanServer");

core-spi-test/src/main/java/org/ehcache/internal/store/StoreBulkComputeTest.java

@@ -81,8 +81,8 @@ public void remappingFunctionReturnsIterableOfEntriesForEachInputEntry() throws
     try {
       Map<K, Store.ValueHolder<V>> mapFromRemappingFunction = kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
-          update.put(entry.getKey(), entry.getValue());
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
+          update.put(entry.getKey(), entry.getValue().get());
         }
         return update.entrySet();
       }
@@ -134,7 +134,7 @@ public void mappingIsRemovedFromStoreForNullValueEntriesFromRemappingFunction()
     try {
       kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
           update.put(entry.getKey(), null);
         }
         return update.entrySet();
@@ -167,13 +167,13 @@ public void remappingFunctionGetsIterableWithMappedStoreEntryValueOrNull() throw
     try {
       kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
           if (mappedEntries.containsKey(entry.getKey())) {
-            assertThat(entry.getValue(), is(mappedEntries.get(entry.getKey())));
+            assertThat(entry.getValue().get(), is(mappedEntries.get(entry.getKey())));
           } else {
             assertThat(entry.getValue(), is(nullValue()));
           }
-          update.put(entry.getKey(), entry.getValue());
+          update.put(entry.getKey(), entry.getValue().get());
         }
         return update.entrySet();
       }
@@ -199,7 +199,7 @@ public void computeValuesForEveryKeyUsingARemappingFunction() throws Exception {
     try {
       kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
           update.put(entry.getKey(), computedEntries.get(entry.getKey()));
         }
         return update.entrySet();
@@ -230,7 +230,7 @@ public void remappingFunctionProducesWrongKeyType() throws Exception {
     try {
       kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
           if (factory.getKeyType() == String.class) {
             update.put((K)new StringBuffer(entry.getKey().toString()), computedEntries.get(entry.getKey()));
           } else {
@@ -264,7 +264,7 @@ public void remappingFunctionProducesWrongValueType() throws Exception {
     try {
       kvStore.bulkCompute(inputKeys, entries -> {
         Map<K, V> update = new HashMap<>();
-        for (Map.Entry<? extends K, ? extends V> entry : entries) {
+        for (Map.Entry<? extends K, ? extends Store.ValueHolder<V>> entry : entries) {
           if (factory.getKeyType() == String.class) {
             update.put(entry.getKey(), (V)new StringBuffer(computedEntries.get(entry.getKey()).toString()));
           } else {

core-spi-test/src/main/java/org/ehcache/internal/store/StoreGetAndComputeTest.java

@@ -226,7 +226,7 @@ public void testExceptionOnSupplier() throws Exception {
       kvStore.put(key, value);
       assertThat(kvStore.get(key).get(), is(value));
 
-      kvStore.computeAndGet(key, (keyParam, oldValue) -> oldValue, () -> { throw re; }, () -> false);
+      kvStore.computeAndGet(key, (keyParam, oldValue) -> oldValue.get(), () -> { throw re; }, () -> false);
     } catch (StoreAccessException e) {
       assertThat(e.getCause(), is(re));
     }
@@ -248,7 +248,7 @@ public void testPassThroughExceptionOnSupplier() throws Exception {
       kvStore.put(key, value);
       assertThat(kvStore.get(key).get(), is(value));
 
-      kvStore.computeAndGet(key, (keyParam, oldValue) -> oldValue, () -> { throw re; }, () -> false);
+      kvStore.computeAndGet(key, (keyParam, oldValue) -> oldValue.get(), () -> { throw re; }, () -> false);
     } catch (RuntimeException e) {
       assertThat(e, is(exception));
     }
@@ -268,7 +268,7 @@ public void testComputeExpiresOnAccess() throws Exception {
     try {
       kvStore.put(key, value);
 
-      Store.ValueHolder<V> result = kvStore.getAndCompute(key, (k, v) -> v);
+      Store.ValueHolder<V> result = kvStore.getAndCompute(key, (k, v) -> v.get());
       assertThat(result.get(), is(value));
       assertThat(kvStore.get(key), nullValue());
     } catch (StoreAccessException e) {

ehcache-107/build.gradle

@@ -109,6 +109,7 @@ task tckTest(type: Test, dependsOn: unpackTckTests) {
   reports.junitXml.destination = file("$buildDir/tck-tests-results")
   reports.html.destination = file("$buildDir/reports/tck-tests")
 
+  systemProperty 'org.jsr107.tck.support.server.address', '127.0.0.1'
   systemProperty 'java.net.preferIPv4Stack', 'true'
   systemProperty 'javax.management.builder.initial', 'org.ehcache.jsr107.internal.tck.Eh107MBeanServerBuilder'
   systemProperty 'org.jsr107.tck.management.agentId', 'Eh107MBeanServer'

ehcache-core/src/main/java/org/ehcache/core/Ehcache.java

@@ -183,15 +183,14 @@ public void compute(K key, final BiFunction<? super K, ? super V, ? extends V> c
       getObserver.begin();
 
       try {
-        BiFunction<K, V, V> fn = (mappedKey, mappedValue) -> {
+        BiFunction<K, ValueHolder<V>, V> fn = (mappedKey, mappedValue) -> {
           if (mappedValue == null) {
             getObserver.end(GetOutcome.MISS);
+            return computeFunction.apply(mappedKey, null);
           } else {
             getObserver.end(GetOutcome.HIT);
+            return computeFunction.apply(mappedKey, mappedValue.get());
           }
-
-          return computeFunction.apply(mappedKey, mappedValue);
-
         };
 
         ValueHolder<V> compute = store.computeAndGet(key, fn, replaceEqual, invokeWriter);
@@ -259,7 +258,7 @@ public V getAndPut(K key, final V value) {
 
   // The compute function that will return the keys to their NEW values, taking the keys to their old values as input;
   // but this could happen in batches, i.e. not necessary containing all of the entries of the Iterable passed to this method
-  public static class PutAllFunction<K, V> implements Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> {
+  public static class PutAllFunction<K, V> implements Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> {
 
     private final Logger logger;
     private final Map<K, V> entriesToRemap;
@@ -274,13 +273,13 @@ public PutAllFunction(Logger logger, Map<K, V> entriesToRemap, ExpiryPolicy<? su
     }
 
     @Override
-    public Iterable<? extends Map.Entry<? extends K, ? extends V>> apply(final Iterable<? extends Map.Entry<? extends K, ? extends V>> entries) {
+    public Iterable<? extends Map.Entry<? extends K, ? extends V>> apply(final Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>> entries) {
       Map<K, V> mutations = new LinkedHashMap<>();
 
       // then record we handled these mappings
-      for (Map.Entry<? extends K, ? extends V> entry: entries) {
+      for (Map.Entry<? extends K, ? extends ValueHolder<V>> entry: entries) {
         K key = entry.getKey();
-        V existingValue = entry.getValue();
+        ValueHolder<V> existingValue = entry.getValue();
         V newValue = entriesToRemap.remove(key);
 
         if (newValueAlreadyExpired(key, existingValue, newValue)) {
@@ -302,7 +301,7 @@ public Map<K, V> getEntriesToRemap() {
       return entriesToRemap;
     }
 
-    private boolean newValueAlreadyExpired(K key, V oldValue, V newValue) {
+    private boolean newValueAlreadyExpired(K key, ValueHolder<V> oldValue, V newValue) {
       return EhcacheBase.newValueAlreadyExpired(logger, expiry, key, oldValue, newValue);
     }
 
@@ -315,17 +314,17 @@ public AtomicInteger getActualUpdateCount() {
     }
   }
 
-  public static class RemoveAllFunction<K, V> implements Function<Iterable<? extends Map.Entry<? extends K, ? extends V>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> {
+  public static class RemoveAllFunction<K, V> implements Function<Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>>, Iterable<? extends Map.Entry<? extends K, ? extends V>>> {
 
     private final AtomicInteger actualRemoveCount = new AtomicInteger();
 
     @Override
-    public Iterable<? extends Map.Entry<? extends K, ? extends V>> apply(final Iterable<? extends Map.Entry<? extends K, ? extends V>> entries) {
+    public Iterable<? extends Map.Entry<? extends K, ? extends V>> apply(final Iterable<? extends Map.Entry<? extends K, ? extends ValueHolder<V>>> entries) {
       Map<K, V> results = new LinkedHashMap<>();
 
-      for (Map.Entry<? extends K, ? extends V> entry : entries) {
+      for (Map.Entry<? extends K, ? extends ValueHolder<V>> entry : entries) {
         K key = entry.getKey();
-        V existingValue = entry.getValue();
+        ValueHolder<V> existingValue = entry.getValue();
 
         if (existingValue != null) {
           actualRemoveCount.incrementAndGet();

ehcache-core/src/main/java/org/ehcache/core/EhcacheBase.java

@@ -385,7 +385,7 @@ public void putAll(Map<? extends K, ? extends V> entries) throws BulkCacheWritin
 
   protected abstract void doPutAll(Map<? extends K, ? extends V> entries) throws StoreAccessException, BulkCacheWritingException;
 
-  protected boolean newValueAlreadyExpired(K key, V oldValue, V newValue) {
+  protected boolean newValueAlreadyExpired(K key, ValueHolder<V> oldValue, V newValue) {
     return newValueAlreadyExpired(logger, runtimeConfiguration.getExpiryPolicy(), key, oldValue, newValue);
   }
 
@@ -424,7 +424,7 @@ public void removeAll(Set<? extends K> keys) throws BulkCacheWritingException {
 
   protected abstract void doRemoveAll(Set<? extends K> keys) throws BulkCacheWritingException, StoreAccessException;
 
-  protected static <K, V> boolean newValueAlreadyExpired(Logger logger, ExpiryPolicy<? super K, ? super V> expiry, K key, V oldValue, V newValue) {
+  protected static <K, V> boolean newValueAlreadyExpired(Logger logger, ExpiryPolicy<? super K, ? super V> expiry, K key, ValueHolder<V> oldValue, V newValue) {
     if (newValue == null) {
       return false;
     }
@@ -434,7 +434,7 @@ protected static <K, V> boolean newValueAlreadyExpired(Logger logger, ExpiryPoli
       if (oldValue == null) {
         duration = expiry.getExpiryForCreation(key, newValue);
       } else {
-        duration = expiry.getExpiryForUpdate(key, () -> oldValue, newValue);
+        duration = expiry.getExpiryForUpdate(key, oldValue, newValue);
       }
     } catch (RuntimeException re) {
       logger.error("Expiry computation caused an exception - Expiry duration will be 0 ", re);
@@ -680,7 +680,7 @@ private void loadAllReplace(Set<? extends K> keys, final Function<Iterable<? ext
       try {
         store.bulkCompute(keys, entries -> {
           Collection<K> keys1 = new ArrayList<>();
-          for (Map.Entry<? extends K, ? extends V> entry : entries) {
+          for (Map.Entry<? extends K, ? extends ValueHolder<V>> entry : entries) {
             keys1.add(entry.getKey());
           }
           return cacheLoaderWriterLoadAllForKeys(keys1, loadFunction).entrySet();