Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from Guava's cache to ConcurrentHashMap in JellyClassLoaderTearOff #215

Merged
merged 2 commits into from
Apr 26, 2021
Merged

Migrate from Guava's cache to ConcurrentHashMap in JellyClassLoaderTearOff #215

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2527058
Migrate from Guava's cache to ConcurrentHashMap in JellyClassLoaderTe…
basil Apr 7, 2021
2a74858
Jesse's review feedback
basil Apr 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions jelly/src/main/java/org/kohsuke/stapler/jelly/JellyClassLoaderTearOff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

package org.kohsuke.stapler.jelly;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import org.apache.commons.jelly.JellyContext;
import org.apache.commons.jelly.JellyException;
import org.apache.commons.jelly.TagLibrary;
Expand All @@ -35,6 +32,8 @@

import java.lang.ref.WeakReference;
import java.net.URL;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

/**
* {@link MetaClassLoader} tear-off for Jelly support.
Expand All @@ -47,7 +46,7 @@ public class JellyClassLoaderTearOff {
/**
* See {@link JellyClassTearOff#scripts} for why we use {@link WeakReference} here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be dead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but this seems to be a preëxisting issue that I am making neither better nor worse in this PR.

*/
private volatile WeakReference<LoadingCache<String,TagLibrary>> taglibs;
private volatile WeakReference<ConcurrentMap<String,TagLibrary>> taglibs;

static ExpressionFactory EXPRESSION_FACTORY = new JexlExpressionFactory();

Expand All @@ -56,16 +55,18 @@ public JellyClassLoaderTearOff(MetaClassLoader owner) {
}

public TagLibrary getTagLibrary(String nsUri) {
LoadingCache<String,TagLibrary> m=null;
ConcurrentMap<String,TagLibrary> m=null;
if(taglibs!=null)
m = taglibs.get();
if(m==null) {
m = CacheBuilder.newBuilder().build(new CacheLoader<String,TagLibrary>() {
public TagLibrary load(String nsUri) {
m = new ConcurrentHashMap<>();
taglibs = new WeakReference<>(m);
}
TagLibrary tl = m.computeIfAbsent(nsUri, key -> {
if(owner.parent!=null) {
// parent first
TagLibrary tl = owner.parent.loadTearOff(JellyClassLoaderTearOff.class).getTagLibrary(nsUri);
if(tl!=null) return tl;
TagLibrary taglib = owner.parent.loadTearOff(JellyClassLoaderTearOff.class).getTagLibrary(nsUri);
if(taglib!=null) return taglib;
}

String taglibBasePath = trimHeadSlash(nsUri);
Expand All @@ -90,12 +91,7 @@ public TagLibrary load(String nsUri) {
return new StaplerTagLibrary();

return NO_SUCH_TAGLIBRARY; // "not found" is also cached.
}
});
taglibs = new WeakReference<LoadingCache<String,TagLibrary>>(m);
}

TagLibrary tl = m.getUnchecked(nsUri);
});
if (tl==NO_SUCH_TAGLIBRARY) return null;
return tl;
}
Expand Down