Skip to content

Commit

Permalink
Replace Guava cache with Caffeine (#132)
Browse files Browse the repository at this point in the history
* Use weak keys/values for FlowNode cache

* Replace guava cache with Caffeine
* Use weak keys and values instead of soft values which are causing
  unpredictibles GC pauses.

* Remove explicit version; rely on BOM

* Switch back to soft values
  • Loading branch information
Vlatombe authored Jan 20, 2022
1 parent bd7c3d2 commit fc82a38
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 13 deletions.
4 changes: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>caffeine-api</artifactId>
</dependency>
<dependency>
<!-- Required for running with Java 9+ -->
<groupId>org.jboss.marshalling</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@

package org.jenkinsci.plugins.workflow.support.storage;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.thoughtworks.xstream.converters.Converter;
import com.thoughtworks.xstream.converters.MarshallingContext;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.core.JVM;
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
import java.nio.file.NoSuchFileException;
import java.util.concurrent.CompletionException;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.actions.FlowNodeAction;
Expand All @@ -48,7 +49,6 @@
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.file.NoSuchFileException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -57,8 +57,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.logging.Level;
import java.util.logging.Logger;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
Expand All @@ -71,11 +69,10 @@
public class SimpleXStreamFlowNodeStorage extends FlowNodeStorage {
private final File dir;
private final FlowExecution exec;
private final LoadingCache<String,FlowNode> nodeCache = CacheBuilder.newBuilder().softValues().build(new CacheLoader<String,FlowNode>() {
@Override public FlowNode load(String key) throws Exception {
return SimpleXStreamFlowNodeStorage.this.load(key).node;
}
});

private final LoadingCache<String,FlowNode> nodeCache = Caffeine.newBuilder()
.softValues()
.build(key -> SimpleXStreamFlowNodeStorage.this.load(key).node);

private static final Logger LOGGER = Logger.getLogger(SimpleXStreamFlowNodeStorage.class.getName());

Expand All @@ -99,7 +96,7 @@ public FlowNode getNode(String id) throws IOException {
}
}
return nodeCache.get(id);
} catch (ExecutionException x) {
} catch (CompletionException x) {
Throwable cause = x.getCause();
if (cause instanceof NoSuchFileException) {
LOGGER.finer("Tried to load FlowNode where file does not exist, for id "+id);
Expand Down Expand Up @@ -254,7 +251,7 @@ private void storeActions() { // We've already loaded the actions, may as well
XSTREAM.registerConverter(new Converter() {
private final RobustReflectionConverter ref = new RobustReflectionConverter(XSTREAM.getMapper(), JVM.newReflectionProvider());
// IdentityHashMap could leak memory. WeakHashMap compares by equals, which will fail with NPE in FlowNode.hashCode.
private final Map<FlowNode,String> ids = CacheBuilder.newBuilder().weakKeys().<FlowNode,String>build().asMap();
private final Map<FlowNode,String> ids = Caffeine.newBuilder().weakKeys().<FlowNode,String>build().asMap();
@Override public boolean canConvert(Class type) {
return FlowNode.class.isAssignableFrom(type);
}
Expand Down

0 comments on commit fc82a38

Please sign in to comment.