Skip to content

Commit

Permalink
Merge pull request #220 from jglick/no-guava
Browse files Browse the repository at this point in the history
Replacing Guava’s `CacheBuilder` with Java Platform equivalents
  • Loading branch information
jglick authored May 8, 2021
2 parents a6dd7ae + 3338be1 commit fcf95ad
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 103 deletions.
5 changes: 0 additions & 5 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@
<version>1.8.3</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>11.0.1</version>
</dependency>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>asm5</artifactId>
Expand Down
38 changes: 25 additions & 13 deletions core/src/main/java/org/kohsuke/stapler/AbstractTearOff.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@

package org.kohsuke.stapler;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

import java.io.File;
import java.lang.ref.Reference;
import java.lang.ref.SoftReference;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
Expand All @@ -52,13 +54,13 @@ public abstract class AbstractTearOff<CLT,S,E extends Exception> extends Caching

private static final class ExpirableCacheHit<S> {
private final long timestamp;
private final S script;
private final Reference<S> script;
ExpirableCacheHit(long timestamp, S script) {
this.timestamp = timestamp;
this.script = script;
this.script = new SoftReference<>(script);
}
}
private final Cache<URL, ExpirableCacheHit<S>> cachedScripts = CacheBuilder.newBuilder().softValues().build();
private final Map<String, ExpirableCacheHit<S>> cachedScripts = new ConcurrentHashMap<>();

protected AbstractTearOff(MetaClass owner, Class<CLT> cltClass) {
this.owner = owner;
Expand Down Expand Up @@ -122,7 +124,7 @@ public S resolveScript(String name) throws E {
LOGGER.log(Level.FINE, "no timestamp associated with {0}", file);
return parseScript(res);
} else {
ExpirableCacheHit<S> cached = cachedScripts.getIfPresent(res);
ExpirableCacheHit<S> cached = cachedScripts.get(res.toString());
if (cached == null) {
S script;
if (LOGGER.isLoggable(Level.FINE)) {
Expand All @@ -136,15 +138,25 @@ public S resolveScript(String name) throws E {
LOGGER.log(Level.FINE, "cache miss on {0}", res);
script = parseScript(res);
}
cachedScripts.put(res, new ExpirableCacheHit<>(timestamp, script));
cachedScripts.put(res.toString(), new ExpirableCacheHit<>(timestamp, script));
return script;
} else if (timestamp == cached.timestamp) {
LOGGER.log(Level.FINE, "cache hit on {0}", res);
return cached.script;
} else {
LOGGER.log(Level.FINE, "expired cache hit on {0}", res);
S script = parseScript(res);
cachedScripts.put(res, new ExpirableCacheHit<>(timestamp, script));
S script;
if (timestamp == cached.timestamp) {
script = cached.script.get();
if (script == null) {
LOGGER.log(Level.FINE, "cache hit on {0} but value collected", res);
} else {
LOGGER.log(Level.FINE, "cache hit on {0}", res);
}
} else {
LOGGER.log(Level.FINE, "expired cache hit on {0}", res);
script = null;
}
if (script == null) {
script = parseScript(res);
cachedScripts.put(res.toString(), new ExpirableCacheHit<>(timestamp, script));
}
return script;
}
}
Expand Down
47 changes: 25 additions & 22 deletions core/src/main/java/org/kohsuke/stapler/CachingScriptLoader.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package org.kohsuke.stapler;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;

import java.lang.ref.Reference;
import java.lang.ref.SoftReference;
import java.net.URL;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

/**
* Convenient base class for caching loaded scripts.
Expand All @@ -22,20 +23,8 @@ public abstract class CachingScriptLoader<S, E extends Exception> {
*
* So it's important to allow Scripts to be garbage collected.
* This is not an ideal fix, but it works.
*
* {@link Optional} is used as Google Collection doesn't allow null values in a map.
*/
private final LoadingCache<String,Optional<S>> scripts = CacheBuilder.newBuilder().softValues().build(new CacheLoader<String, Optional<S>>() {
public Optional<S> load(String from) {
try {
return Optional.create(loadScript(from));
} catch (RuntimeException e) {
throw e; // pass through
} catch (Exception e) {
throw new ScriptLoadException(e);
}
}
});
private final Map<String, Optional<Reference<S>>> scripts = new ConcurrentHashMap<>();

/**
* Locates the view script of the given name.
Expand All @@ -56,10 +45,24 @@ public Optional<S> load(String from) {
* @return null if none was found.
*/
public S findScript(String name) throws E {
if (MetaClass.NO_CACHE)
if (MetaClass.NO_CACHE) {
return loadScript(name);
else
return scripts.getUnchecked(name).get();
} else {
Optional<Reference<S>> sr = scripts.get(name);
S s;
if (sr == null) { // never before computed
s = null;
} else if (!sr.isPresent()) { // cached as null
return null;
} else { // cached as non-null; may or may not still have value
s = sr.get().get();
}
if (s == null) { // needs to be computed
s = loadScript(name);
scripts.put(name, s == null ? Optional.empty() : Optional.of(new SoftReference<>(s)));
}
return s;
}
}

/**
Expand All @@ -70,8 +73,8 @@ public S findScript(String name) throws E {
/**
* Discards the cached script.
*/
public synchronized void clearScripts() {
scripts.invalidateAll();
public void clearScripts() {
scripts.clear();
}

protected abstract URL getResource(String name);
Expand Down
15 changes: 6 additions & 9 deletions core/src/main/java/org/kohsuke/stapler/Function.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

package org.kohsuke.stapler;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.interceptor.Interceptor;
import org.kohsuke.stapler.interceptor.InterceptorAnnotation;
Expand All @@ -38,7 +35,6 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.WrongMethodTypeException;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Executable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
Expand Down Expand Up @@ -195,7 +191,7 @@ Object bindAndInvoke(Object o, StaplerRequest req, StaplerResponse rsp, Object..
}

// if the databinding method is provided, call that
Function binder = PARSE_METHODS.getUnchecked(t);
Function binder = PARSE_METHODS.get(t);
if (binder!=RETURN_NULL) {
arguments[i] = binder.bindAndInvoke(null,req,rsp);
continue;
Expand All @@ -216,7 +212,7 @@ Object bindAndInvoke(Object o, StaplerRequest req, StaplerResponse rsp, Object..
* Computing map that discovers the static 'fromStapler' method from a class.
* The discovered method will be returned as a Function so that the invocation can do parameter injections.
*/
private static final LoadingCache<Class,Function> PARSE_METHODS;
private static final ClassValue<Function> PARSE_METHODS;
private static final Function RETURN_NULL;

static {
Expand All @@ -226,8 +222,9 @@ Object bindAndInvoke(Object o, StaplerRequest req, StaplerResponse rsp, Object..
throw new AssertionError(e); // impossible
}

PARSE_METHODS = CacheBuilder.newBuilder().weakKeys().build(new CacheLoader<Class,Function>() {
public Function load(Class from) {
PARSE_METHODS = new ClassValue<Function>() {
@Override
public Function computeValue(Class<?> from) {
// MethdFunction for invoking a static method as a static method
FunctionList methods = new ClassDescriptor(from).methods.name("fromStapler");
switch (methods.size()) {
Expand Down Expand Up @@ -259,7 +256,7 @@ public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object..
};
}
}
});
};
}

/**
Expand Down
51 changes: 0 additions & 51 deletions core/src/main/java/org/kohsuke/stapler/Optional.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@
package org.kohsuke.stapler;

/**
* Indicates a failure to load a script.
*
* @author Kohsuke Kawaguchi
* @deprecated No longer used.
*/
@Deprecated
public class ScriptLoadException extends RuntimeException {
public ScriptLoadException(String message, Throwable cause) {
super(message, cause);
Expand Down

0 comments on commit fcf95ad

Please sign in to comment.