[
https://issues.apache.org/jira/browse/OPENJPA-2529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14137782#comment-14137782
]
Matt Bishop commented on OPENJPA-2529:
--------------------------------------
I can't find any way of submitting a patch file, but it's not big:
diff --git
a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
index 1478c3a..e5ebf4b 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
@@ -30,6 +30,7 @@ import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReentrantLock;
import org.apache.commons.collections.map.LinkedMap;
@@ -81,6 +82,7 @@ public class QueryImpl
implements Query {
private static final Localizer _loc =
Localizer.forPackage(QueryImpl.class);
+ private static final Map<String, Class> _classCache = new
ConcurrentHashMap<String, Class>();
private final String _language;
private final StoreQuery _storeQuery;
@@ -1687,8 +1689,16 @@ public class QueryImpl
if (_loader == null)
_loader = _broker.getConfiguration().getClassResolverInstance().
getClassLoader(_class, _broker.getClassLoader());
+ Class cachedClass = _classCache.get(name);
+ if (cachedClass != null) {
+ return cachedClass;
+ }
try {
- return Strings.toClass(name, _loader);
+ Class loadedClass = Strings.toClass(name, _loader);
+ if (loadedClass != null) {
+ _classCache.put(name, loadedClass);
+ }
+ return loadedClass;
} catch (RuntimeException re) {
} catch (NoClassDefFoundError ncdfe) {
}
> QueryImpl.toClass() causes thousands of BLOCKED threads under load
> ------------------------------------------------------------------
>
> Key: OPENJPA-2529
> URL: https://issues.apache.org/jira/browse/OPENJPA-2529
> Project: OpenJPA
> Issue Type: Improvement
> Components: kernel
> Affects Versions: 2.3.0
> Environment: Java7u51 Linux (Centos?)
> Reporter: Matt Bishop
> Priority: Minor
> Labels: query-parser
>
> When evaluating a JPQL query, the kernel eventually finds it's way to
> QueryImpl.toClass(string) to provide a Class for a given name. This delegates
> to Serp, which calls Class.forName(string) (I'm leaving out the class loaders
> here).
> The problem is that under heavy load, with lots of threads, the calls to
> Class.forName() BLOCK as that method has to get the class loader lock before
> finding/loading the named class. In my perf tests, which executes 500 threads
> against JPA and takes a thread dump every 10 seconds, I find about 2400
> BLOCKED threads at exactly the same place:
> "http-bio-8080-exec-16" daemon prio=10 tid=0x00007fc95c503000 nid=0x2eb
> waiting for monitor entry [0x00007fc98034d000]
> java.lang.Thread.State: BLOCKED (on object monitor)
> at java.lang.Class.forName0(Native Method)
> at java.lang.Class.forName(Class.java:270)
> at serp.util.Strings.toClass(Strings.java:162)
> at serp.util.Strings.toClass(Strings.java:108)
> at org.apache.openjpa.kernel.QueryImpl.toClass(QueryImpl.java:1698)
> at org.apache.openjpa.kernel.QueryImpl.classForName(QueryImpl.java:1653)
> at
> org.apache.openjpa.kernel.ExpressionStoreQuery$1.classForName(ExpressionStoreQuery.java:113)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getPathOrConstant(JPQLExpressionBuilder.java:1883)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.eval(JPQLExpressionBuilder.java:1189)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getValue(JPQLExpressionBuilder.java:2095)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getValue(JPQLExpressionBuilder.java:2081)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.eval(JPQLExpressionBuilder.java:1137)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getExpression(JPQLExpressionBuilder.java:2011)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.eval(JPQLExpressionBuilder.java:1059)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getExpression(JPQLExpressionBuilder.java:2011)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.eval(JPQLExpressionBuilder.java:1001)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.evalWhereClause(JPQLExpressionBuilder.java:672)
> at
> org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.getQueryExpressions(JPQLExpressionBuilder.java:297)
> at org.apache.openjpa.kernel.jpql.JPQLParser.eval(JPQLParser.java:67)
> at
> org.apache.openjpa.kernel.ExpressionStoreQuery$DataStoreExecutor.<init>(ExpressionStoreQuery.java:763)
> at
> org.apache.openjpa.kernel.ExpressionStoreQuery.newDataStoreExecutor(ExpressionStoreQuery.java:179)
> at
> org.apache.openjpa.datacache.QueryCacheStoreQuery.newDataStoreExecutor(QueryCacheStoreQuery.java:288)
> at
> org.apache.openjpa.kernel.QueryImpl.createExecutor(QueryImpl.java:752)
> at
> org.apache.openjpa.kernel.QueryImpl.compileForDataStore(QueryImpl.java:710)
> at
> org.apache.openjpa.kernel.QueryImpl.compileForExecutor(QueryImpl.java:692)
> at org.apache.openjpa.kernel.QueryImpl.compile(QueryImpl.java:592)
> at
> org.apache.openjpa.persistence.EntityManagerImpl.createQuery(EntityManagerImpl.java:997)
> at
> org.apache.openjpa.persistence.EntityManagerImpl.createQuery(EntityManagerImpl.java:979)
> My suggestion is to keep a map at QueryImpl from name to Class and avoid
> Class.forName() when the class has already been found once.
> Three issues to consider:
> 1. Classloader. Class caches usually are a bad idea because one can get a
> different Class instance from a different class loader. In QueryImpl, only
> one ClassLoader (_loader) is ever used.
> 2. Concurrency. The class cache needs to be thread-safe, so I used a
> ConcurrentHashMap (JDK version) to store results. I'm ok with multiple
> threads reloading the same class occasionally as they will simply replace the
> stored class with the same class. That is preferable to locking the thread so
> only one mapping can be created ever.
> 3. Class unload. The Class itself may be unloaded, but cannot be
> garbage-collected because the map will retain a reference. I don't think this
> is a real problem given the nature of the Query; the same JQPL tends to be
> executed repeatedly, so it is unlikely that the class will be unloaded. I'm
> sure someone has a different opinion on this.
> The provided patch, which stores found classes in a map, has reduced the
> amount of BLOCKED state threads in my test down to 100.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
