You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by kw...@apache.org on 2007/02/12 03:33:05 UTC
svn commit: r506230 - in /incubator/openjpa/trunk:
openjpa-kernel/src/main/java/org/apache/openjpa/ee/
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
openjpa-kernel/src/main/java/org/apache/openjpa/util/
openjpa-kernel/src/main/resources/org/a...
Author: kwsutter
Date: Sun Feb 11 18:33:05 2007
New Revision: 506230
URL: http://svn.apache.org/viewvc?view=rev&rev=506230
Log:
OPENJPA-138. Some updates to help with performance of OpenJPA in an application server environment. Details can be found in the OPENJPA-138 Issue.
Modified:
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java
incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
@@ -29,6 +29,7 @@
implements ManagedRuntime {
private String _tmLoc = "java:/TransactionManager";
+ private static TransactionManager _tm;
/**
* Return the location of the {@link TransactionManager} in JNDI.
@@ -44,13 +45,18 @@
_tmLoc = name;
}
- public TransactionManager getTransactionManager()
- throws Exception {
- Context ctx = new InitialContext();
- try {
- return (TransactionManager) ctx.lookup(_tmLoc);
- } finally {
- ctx.close();
+ /**
+ * Return the cached TransactionManager instance.
+ */
+ public TransactionManager getTransactionManager() throws Exception {
+ if (_tm == null) {
+ Context ctx = new InitialContext();
+ try {
+ _tm = (TransactionManager) ctx.lookup(_tmLoc);
+ } finally {
+ ctx.close();
+ }
}
- }
+ return _tm;
+ }
}
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java Sun Feb 11 18:33:05 2007
@@ -64,7 +64,8 @@
// configuration
private final OpenJPAConfiguration _conf;
private transient boolean _readOnly = false;
- private transient RuntimeException _closed = null;
+ private transient boolean _closed = false;
+ private transient RuntimeException _closedException = null;
private Map _userObjects = null;
// internal lock: spec forbids synchronization on this object
@@ -267,7 +268,7 @@
* Returns true if this broker factory is closed.
*/
public boolean isClosed() {
- return _closed != null;
+ return _closed;
}
public void close() {
@@ -297,7 +298,10 @@
(_conf.getMetaDataRepositoryInstance());
_conf.close();
- _closed = new IllegalStateException();
+ _closed = true;
+ Log log = _conf.getLog(OpenJPAConfiguration.LOG_RUNTIME);
+ if (log.isTraceEnabled())
+ _closedException = new IllegalStateException();
} finally {
unlock();
}
@@ -546,12 +550,18 @@
}
/**
- * Throw an exception if the factory is closed.
+ * Throw an exception if the factory is closed. The exact message and
+ * content of the exception varies whether TRACE is enabled or not.
*/
private void assertOpen() {
- if (_closed != null)
- throw new InvalidStateException(_loc.get("closed-factory")).
- setCause(_closed);
+ if (_closed) {
+ if (_closedException == null) // TRACE not enabled
+ throw new InvalidStateException(_loc
+ .get("closed-factory-notrace"));
+ else
+ throw new InvalidStateException(_loc.get("closed-factory"))
+ .setCause(_closedException);
+ }
}
////////////////////
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java Sun Feb 11 18:33:05 2007
@@ -63,6 +63,7 @@
import org.apache.openjpa.lib.util.ReferenceHashMap;
import org.apache.openjpa.lib.util.ReferenceHashSet;
import org.apache.openjpa.lib.util.ReferenceMap;
+import org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
import org.apache.openjpa.lib.util.concurrent.ReentrantLock;
import org.apache.openjpa.meta.ClassMetaData;
import org.apache.openjpa.meta.FieldMetaData;
@@ -138,6 +139,9 @@
private static final Localizer _loc =
Localizer.forPackage(BrokerImpl.class);
+ // Cache for from/to type assignments
+ private static ConcurrentReferenceHashMap _assignableTypes =
+ new ConcurrentReferenceHashMap(ReferenceMap.HARD, ReferenceMap.WEAK);
// the store manager in use; this may be a decorator such as a
// data cache store manager around the native store manager
@@ -215,7 +219,8 @@
// status
private int _flags = 0;
- private RuntimeException _closed = null;
+ private boolean _closed = false;
+ private RuntimeException _closedException = null;
// event managers
private TransactionEventManager _transEventManager = null;
@@ -1096,8 +1101,7 @@
cls));
return PCRegistry.newObjectId(cls, (String) val);
}
-
- if (meta.getObjectIdType().isAssignableFrom(val.getClass())) {
+ if (isAssignable(meta.getObjectIdType(), val.getClass())) {
if (!meta.isOpenJPAIdentity() && meta.isObjectIdTypeShared())
return new ObjectId(cls, val);
return val;
@@ -1119,6 +1123,37 @@
}
/**
+ * Cache from/to assignments to avoid Class.isAssignableFrom overhead
+ * @param from the target Class
+ * @param to the Class to test
+ * @return true if the "to" class could be assigned to "from" class
+ */
+ private boolean isAssignable(Class from, Class to) {
+ boolean isAssignable;
+ ConcurrentReferenceHashMap assignableTo =
+ (ConcurrentReferenceHashMap) _assignableTypes.get(from);
+
+ if (assignableTo != null) { // "to" cache exists...
+ isAssignable = (assignableTo.get(to) != null);
+ if (!isAssignable) { // not in the map yet...
+ isAssignable = from.isAssignableFrom(to);
+ if (isAssignable) {
+ assignableTo.put(to, new Object());
+ }
+ }
+ } else { // no "to" cache yet...
+ isAssignable = from.isAssignableFrom(to);
+ if (isAssignable) {
+ assignableTo = new ConcurrentReferenceHashMap(
+ ReferenceMap.HARD, ReferenceMap.WEAK);
+ _assignableTypes.put(from, assignableTo);
+ assignableTo.put(to, new Object());
+ }
+ }
+ return isAssignable;
+ }
+
+ /**
* Create a new state manager for the given oid.
*/
private StateManagerImpl newStateManagerImpl(Object oid, boolean copy) {
@@ -3969,11 +4004,11 @@
///////////
public boolean isClosed() {
- return _closed != null;
+ return _closed;
}
public boolean isCloseInvoked() {
- return _closed != null || (_flags & FLAG_CLOSE_INVOKED) != 0;
+ return _closed || (_flags & FLAG_CLOSE_INVOKED) != 0;
}
public void close() {
@@ -4055,8 +4090,10 @@
_lm.close();
_store.close();
- _closed = new IllegalStateException();
_flags = 0;
+ _closed = true;
+ if (_log.isTraceEnabled())
+ _closedException = new IllegalStateException();
if (err != null)
throw err;
@@ -4246,11 +4283,19 @@
/////////
// Utils
/////////
-
+ /**
+ * Throw an exception if the context is closed. The exact message and
+ * content of the exception varies whether TRACE is enabled or not.
+ */
public void assertOpen() {
- if (_closed != null)
- throw new InvalidStateException(_loc.get("closed"), _closed).
- setFatal(true);
+ if (_closed) {
+ if (_closedException == null) // TRACE not enabled
+ throw new InvalidStateException(_loc.get("closed-notrace"))
+ .setFatal(true);
+ else
+ throw new InvalidStateException(_loc.get("closed"),
+ _closedException).setFatal(true);
+ }
}
public void assertActiveTransaction() {
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java Sun Feb 11 18:33:05 2007
@@ -36,6 +36,8 @@
import org.apache.openjpa.lib.rop.SimpleResultList;
import org.apache.openjpa.lib.rop.WindowResultList;
import org.apache.openjpa.lib.util.Localizer;
+import org.apache.openjpa.lib.util.ReferenceMap;
+import org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
import org.apache.openjpa.meta.ClassMetaData;
import org.apache.openjpa.meta.FetchGroup;
import org.apache.openjpa.meta.FieldMetaData;
@@ -58,6 +60,10 @@
private static final Localizer _loc = Localizer.forPackage
(FetchConfigurationImpl.class);
+ // Cache the from/to isAssignable invocations
+ private static ConcurrentReferenceHashMap _assignableTypes =
+ new ConcurrentReferenceHashMap(ReferenceMap.HARD, ReferenceMap.WEAK);
+
/**
* Configurable state shared throughout a traversal chain.
*/
@@ -613,11 +619,37 @@
}
/**
- * Whether either of the two types is assignable from the other.
+ * Whether either of the two types is assignable from the other. Optimize
+ * for the repeat calls with similar parameters by caching the from/to
+ * type parameters.
*/
- private static boolean isAssignable(Class c1, Class c2) {
- return c1 != null && c2 != null
- && (c1.isAssignableFrom(c2) || c2.isAssignableFrom(c1));
+ private static boolean isAssignable(Class from, Class to) {
+ boolean isAssignable;
+
+ if (from == null || to == null)
+ return false;
+ ConcurrentReferenceHashMap assignableTo =
+ (ConcurrentReferenceHashMap) _assignableTypes.get(from);
+
+ if (assignableTo != null) { // "to" cache exists...
+ isAssignable = (assignableTo.get(to) != null);
+ if (!isAssignable) { // not in the map yet...
+ isAssignable = from.isAssignableFrom(to);
+ if (isAssignable) {
+ assignableTo.put(to, new Object());
+ }
+ }
+ } else { // no "to" cache yet...
+ isAssignable = from.isAssignableFrom(to);
+ if (isAssignable) {
+ assignableTo = new ConcurrentReferenceHashMap(
+ ReferenceMap.HARD, ReferenceMap.WEAK);
+ _assignableTypes.put(from, assignableTo);
+ assignableTo.put(to, new Object());
+ }
+ }
+
+ return isAssignable;
}
/**
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java Sun Feb 11 18:33:05 2007
@@ -17,6 +17,10 @@
import java.io.Serializable;
+import org.apache.openjpa.lib.util.ReferenceHashSet;
+import org.apache.openjpa.lib.util.ReferenceMap;
+import org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
+
/**
* Identity class extended by builtin OpenJPA identity objects.
*
@@ -31,6 +35,9 @@
// type has his based on the least-derived non-object class so that
// user-given ids with non-exact types match ids with exact types
private transient int _typeHash = 0;
+ // cache the types' generated hashcodes
+ private static ConcurrentReferenceHashMap _typeCache =
+ new ConcurrentReferenceHashMap(ReferenceMap.HARD, ReferenceMap.WEAK);
protected OpenJPAId() {
}
@@ -82,13 +89,25 @@
*/
protected abstract boolean idEquals(OpenJPAId other);
+ /**
+ * Generate the hashcode for this Id. Cache the type's generated hashcode
+ * so that it doesn't have to be generated each time.
+ */
public int hashCode() {
if (_typeHash == 0) {
- Class base = type;
- while (base.getSuperclass() != null
- && base.getSuperclass() != Object.class)
- base = base.getSuperclass();
- _typeHash = base.hashCode();
+ Integer typeHashInt = (Integer) _typeCache.get(type);
+ if (typeHashInt == null) {
+ Class base = type;
+ Class superclass = base.getSuperclass();
+ while (superclass != null && superclass != Object.class) {
+ base = base.getSuperclass();
+ superclass = base.getSuperclass();
+ }
+ _typeHash = base.hashCode();
+ _typeCache.put(type, new Integer(_typeHash));
+ } else {
+ _typeHash = typeHashInt.intValue();
+ }
}
return _typeHash ^ idHash();
}
Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties Sun Feb 11 18:33:05 2007
@@ -94,8 +94,13 @@
active: This operation cannot be performed while a Transaction is active.
closed: The context has been closed. The stack trace at which the \
context was closed is held in the embedded exception.
+closed-notrace: The context has been closed. The stack trace at which the \
+ context was closed is available if Runtime=TRACE logging is enabled.
closed-factory: The factory has been closed. The stack trace at \
which the factory was closed is held in the embedded exception.
+closed-factory-notrace: The factory has been closed. The stack trace at \
+ which the factory was closed is available if Runtime=TRACE logging is \
+ enabled.
non-trans-read: To perform reads on persistent data outside of a transaction, \
the "NontransactionalRead" property must be set on the Transaction.
non-trans-write: To perform writes on persistent data outside of a \
Modified: incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?view=diff&rev=506230&r1=506229&r2=506230
==============================================================================
--- incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java (original)
+++ incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
@@ -17,6 +17,8 @@
import org.apache.commons.lang.ObjectUtils;
import org.apache.openjpa.lib.util.Localizer;
+import org.apache.openjpa.lib.util.ReferenceMap;
+import org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
/**
* An object {@link Value}.
@@ -28,6 +30,10 @@
private static final Localizer _loc = Localizer.forPackage
(ObjectValue.class);
+ // cache the types' classloader
+ private static ConcurrentReferenceHashMap _classloaderCache =
+ new ConcurrentReferenceHashMap(ReferenceMap.HARD, ReferenceMap.WEAK);
+
private Object _value = null;
public ObjectValue(String prop) {
@@ -81,10 +87,16 @@
* Allow subclasses to instantiate additional plugins. This method does
* not perform configuration.
*/
- public Object newInstance(String clsName, Class type,
- Configuration conf, boolean fatal) {
- return Configurations.newInstance(clsName, this, conf,
- type.getClassLoader(), fatal);
+ public Object newInstance(String clsName, Class type, Configuration conf,
+ boolean fatal) {
+ ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
+ if (cl == null) {
+ cl = type.getClassLoader();
+ if (cl != null) { // System classloader is returned as null
+ _classloaderCache.put(type, cl);
+ }
+ }
+ return Configurations.newInstance(clsName, this, conf, cl, fatal);
}
public Class getValueType() {
RE: svn commit: r506230 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/ee/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/util/ openjpa-kernel/src/main/resources/org/
Posted by Patrick Linskey <pl...@bea.com>.
Yeah, that's pretty much what we do most of the time also. For the code
in question, I don't think that it's relevant though.
-Patrick
--
Patrick Linskey
BEA Systems, Inc.
_______________________________________________________________________
Notice: This email message, together with any attachments, may contain
information of BEA Systems, Inc., its subsidiaries and affiliated
entities, that may be confidential, proprietary, copyrighted and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.
> -----Original Message-----
> From: Dain Sundstrom [mailto:dain@iq80.com]
> Sent: Tuesday, February 27, 2007 1:29 PM
> To: open-jpa-dev@incubator.apache.org
> Subject: Re: svn commit: r506230 - in
> /incubator/openjpa/trunk:
> openjpa-kernel/src/main/java/org/apache/openjpa/ee/
> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
> openjpa-kernel/src/main/java/org/apache/openjpa/util/
> openjpa-kernel/src/main/resources/org/
>
> Not sure if this is relevant, but normally, I use explicit, thread
> and then system.
>
> -dain
>
> On Feb 26, 2007, at 10:52 PM, Marc Prud'hommeaux wrote:
>
> >
> > How about just assigning cl = ClassLoader.getSystemClassLoader()
> > (which will hopefully never be null) when cl is null?
> >
> >
> >
> > On Feb 27, 2007, at 7:29 AM, Patrick Linskey wrote:
> >
> >> Hi,
> >>
> >> I was just looking at ObjectValue, and noticed something odd in
> >> the code
> >> snippet below. It looks like if cl is null, we will end up with
> >> constant
> >> cache misses. So, outside a container, we'll always be doing the
> >> extra
> >> cache overhead, and will never get any benefit from doing so.
> >>
> >> Should we put some sort of placeholder in place for 'cl'
> when it's
> >> null
> >> to disambiguate?
> >>
> >> -Patrick
> >>
> >>> - public Object newInstance(String clsName, Class type,
> >>> - Configuration conf, boolean fatal) {
> >>> - return Configurations.newInstance(clsName, this, conf,
> >>> - type.getClassLoader(), fatal);
> >>> + public Object newInstance(String clsName, Class type,
> >>> Configuration conf,
> >>> + boolean fatal) {
> >>> + ClassLoader cl = (ClassLoader)
> _classloaderCache.get(type);
> >>> + if (cl == null) {
> >>> + cl = type.getClassLoader();
> >>> + if (cl != null) { // System classloader is
> >>> returned as null
> >>> + _classloaderCache.put(type, cl);
> >>> + }
> >>> + }
> >>> + return Configurations.newInstance(clsName, this,
> >>> conf, cl, fatal);
> >>> }
> >>
> >> --
> >> Patrick Linskey
> >> BEA Systems, Inc.
> >>
> >>
> _____________________________________________________________________
> >> __
> >> Notice: This email message, together with any attachments, may
> >> contain
> >> information of BEA Systems, Inc., its subsidiaries and
> >> affiliated
> >> entities, that may be confidential, proprietary, copyrighted
> >> and/or
> >> legally privileged, and is intended solely for the use of the
> >> individual
> >> or entity named in this message. If you are not the intended
> >> recipient,
> >> and have received this message in error, please
> immediately return
> >> this
> >> by email and then delete it.
> >>
> >>> -----Original Message-----
> >>> From: kwsutter@apache.org [mailto:kwsutter@apache.org]
> >>> Sent: Sunday, February 11, 2007 6:33 PM
> >>> To: open-jpa-commits@incubator.apache.org
> >>> Subject: svn commit: r506230 - in /incubator/openjpa/trunk:
> >>> openjpa-kernel/src/main/java/org/apache/openjpa/ee/
> >>> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
> >>> openjpa-kernel/src/main/java/org/apache/openjpa/util/
> >>> openjpa-kernel/src/main/resources/org/a...
> >>>
> >>> Author: kwsutter
> >>> Date: Sun Feb 11 18:33:05 2007
> >>> New Revision: 506230
> >>>
> >>> URL: http://svn.apache.org/viewvc?view=rev&rev=506230
> >>> Log:
> >>> OPENJPA-138. Some updates to help with performance of
> >>> OpenJPA in an application server environment. Details can be
> >>> found in the OPENJPA-138 Issue.
> >>>
> >>> Modified:
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/ee/JNDIManagedRuntime.java
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/AbstractBrokerFactory.java
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/BrokerImpl.java
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/FetchConfigurationImpl.java
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/util/OpenJPAId.java
> >>>
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> >> apache/openjpa/kernel/localizer.properties
> >>>
> >>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> >> penjpa/lib/conf/ObjectValue.java
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/ee/JNDIManagedRuntime.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java?>
> >> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/ee/JNDIManagedRuntime.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
> >>> @@ -29,6 +29,7 @@
> >>> implements ManagedRuntime {
> >>>
> >>> private String _tmLoc = "java:/TransactionManager";
> >>> + private static TransactionManager _tm;
> >>>
> >>> /**
> >>> * Return the location of the {@link TransactionManager} in
> >>> JNDI.
> >>> @@ -44,13 +45,18 @@
> >>> _tmLoc = name;
> >>> }
> >>>
> >>> - public TransactionManager getTransactionManager()
> >>> - throws Exception {
> >>> - Context ctx = new InitialContext();
> >>> - try {
> >>> - return (TransactionManager) ctx.lookup(_tmLoc);
> >>> - } finally {
> >>> - ctx.close();
> >>> + /**
> >>> + * Return the cached TransactionManager instance.
> >>> + */
> >>> + public TransactionManager getTransactionManager() throws
> >>> Exception {
> >>> + if (_tm == null) {
> >>> + Context ctx = new InitialContext();
> >>> + try {
> >>> + _tm = (TransactionManager) ctx.lookup(_tmLoc);
> >>> + } finally {
> >>> + ctx.close();
> >>> + }
> >>> }
> >>> - }
> >>> + return _tm;
> >>> + }
> >>> }
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/AbstractBrokerFactory.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/java/org/apache/openjpa/kernel/
> >> AbstractBrokerFactory.java
> >> ?> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/AbstractBrokerFactory.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/AbstractBrokerFactory.java Sun Feb 11
> 18:33:05 2007
> >>> @@ -64,7 +64,8 @@
> >>> // configuration
> >>> private final OpenJPAConfiguration _conf;
> >>> private transient boolean _readOnly = false;
> >>> - private transient RuntimeException _closed = null;
> >>> + private transient boolean _closed = false;
> >>> + private transient RuntimeException _closedException = null;
> >>> private Map _userObjects = null;
> >>>
> >>> // internal lock: spec forbids synchronization on this object
> >>> @@ -267,7 +268,7 @@
> >>> * Returns true if this broker factory is closed.
> >>> */
> >>> public boolean isClosed() {
> >>> - return _closed != null;
> >>> + return _closed;
> >>> }
> >>>
> >>> public void close() {
> >>> @@ -297,7 +298,10 @@
> >>> (_conf.getMetaDataRepositoryInstance());
> >>>
> >>> _conf.close();
> >>> - _closed = new IllegalStateException();
> >>> + _closed = true;
> >>> + Log log = _conf.getLog
> >>> (OpenJPAConfiguration.LOG_RUNTIME);
> >>> + if (log.isTraceEnabled())
> >>> + _closedException = new IllegalStateException();
> >>> } finally {
> >>> unlock();
> >>> }
> >>> @@ -546,12 +550,18 @@
> >>> }
> >>>
> >>> /**
> >>> - * Throw an exception if the factory is closed.
> >>> + * Throw an exception if the factory is closed. The
> >>> exact message and
> >>> + * content of the exception varies whether TRACE is
> >>> enabled or not.
> >>> */
> >>> private void assertOpen() {
> >>> - if (_closed != null)
> >>> - throw new
> >>> InvalidStateException(_loc.get("closed-factory")).
> >>> - setCause(_closed);
> >>> + if (_closed) {
> >>> + if (_closedException == null) // TRACE not enabled
> >>> + throw new InvalidStateException(_loc
> >>> + .get("closed-factory-notrace"));
> >>> + else
> >>> + throw new
> >>> InvalidStateException(_loc.get("closed-factory"))
> >>> + .setCause(_closedException);
> >>> + }
> >>> }
> >>>
> >>> ////////////////////
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/BrokerImpl.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?>
> >> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/BrokerImpl.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/BrokerImpl.java Sun Feb 11 18:33:05 2007
> >>> @@ -63,6 +63,7 @@
> >>> import org.apache.openjpa.lib.util.ReferenceHashMap;
> >>> import org.apache.openjpa.lib.util.ReferenceHashSet;
> >>> import org.apache.openjpa.lib.util.ReferenceMap;
> >>> +import
> >>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> >>> import org.apache.openjpa.lib.util.concurrent.ReentrantLock;
> >>> import org.apache.openjpa.meta.ClassMetaData;
> >>> import org.apache.openjpa.meta.FieldMetaData;
> >>> @@ -138,6 +139,9 @@
> >>>
> >>> private static final Localizer _loc =
> >>> Localizer.forPackage(BrokerImpl.class);
> >>> + // Cache for from/to type assignments
> >>> + private static ConcurrentReferenceHashMap _assignableTypes =
> >>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> >>> ReferenceMap.WEAK);
> >>>
> >>> // the store manager in use; this may be a
> >>> decorator such as a
> >>> // data cache store manager around the native store manager
> >>> @@ -215,7 +219,8 @@
> >>>
> >>> // status
> >>> private int _flags = 0;
> >>> - private RuntimeException _closed = null;
> >>> + private boolean _closed = false;
> >>> + private RuntimeException _closedException = null;
> >>>
> >>> // event managers
> >>> private TransactionEventManager _transEventManager = null;
> >>> @@ -1096,8 +1101,7 @@
> >>> cls));
> >>> return PCRegistry.newObjectId(cls, (String) val);
> >>> }
> >>> -
> >>> - if
> >>> (meta.getObjectIdType().isAssignableFrom(val.getClass())) {
> >>> + if (isAssignable(meta.getObjectIdType(),
> >>> val.getClass())) {
> >>> if (!meta.isOpenJPAIdentity() &&
> >>> meta.isObjectIdTypeShared())
> >>> return new ObjectId(cls, val);
> >>> return val;
> >>> @@ -1119,6 +1123,37 @@
> >>> }
> >>>
> >>> /**
> >>> + * Cache from/to assignments to avoid
> >>> Class.isAssignableFrom overhead
> >>> + * @param from the target Class
> >>> + * @param to the Class to test
> >>> + * @return true if the "to" class could be assigned to
> >>> "from" class
> >>> + */
> >>> + private boolean isAssignable(Class from, Class to) {
> >>> + boolean isAssignable;
> >>> + ConcurrentReferenceHashMap assignableTo =
> >>> + (ConcurrentReferenceHashMap)
> _assignableTypes.get(from);
> >>> +
> >>> + if (assignableTo != null) { // "to" cache exists...
> >>> + isAssignable = (assignableTo.get(to) != null);
> >>> + if (!isAssignable) { // not in the map yet...
> >>> + isAssignable = from.isAssignableFrom(to);
> >>> + if (isAssignable) {
> >>> + assignableTo.put(to, new Object());
> >>> + }
> >>> + }
> >>> + } else { // no "to" cache yet...
> >>> + isAssignable = from.isAssignableFrom(to);
> >>> + if (isAssignable) {
> >>> + assignableTo = new ConcurrentReferenceHashMap(
> >>> + ReferenceMap.HARD, ReferenceMap.WEAK);
> >>> + _assignableTypes.put(from, assignableTo);
> >>> + assignableTo.put(to, new Object());
> >>> + }
> >>> + }
> >>> + return isAssignable;
> >>> + }
> >>> +
> >>> + /**
> >>> * Create a new state manager for the given oid.
> >>> */
> >>> private StateManagerImpl newStateManagerImpl(Object oid,
> >>> boolean copy) {
> >>> @@ -3969,11 +4004,11 @@
> >>> ///////////
> >>>
> >>> public boolean isClosed() {
> >>> - return _closed != null;
> >>> + return _closed;
> >>> }
> >>>
> >>> public boolean isCloseInvoked() {
> >>> - return _closed != null || (_flags &
> FLAG_CLOSE_INVOKED) !
> >>> = 0;
> >>> + return _closed || (_flags & FLAG_CLOSE_INVOKED) != 0;
> >>> }
> >>>
> >>> public void close() {
> >>> @@ -4055,8 +4090,10 @@
> >>>
> >>> _lm.close();
> >>> _store.close();
> >>> - _closed = new IllegalStateException();
> >>> _flags = 0;
> >>> + _closed = true;
> >>> + if (_log.isTraceEnabled())
> >>> + _closedException = new IllegalStateException();
> >>>
> >>> if (err != null)
> >>> throw err;
> >>> @@ -4246,11 +4283,19 @@
> >>> /////////
> >>> // Utils
> >>> /////////
> >>> -
> >>> + /**
> >>> + * Throw an exception if the context is closed. The
> >>> exact message and
> >>> + * content of the exception varies whether TRACE is
> >>> enabled or not.
> >>> + */
> >>> public void assertOpen() {
> >>> - if (_closed != null)
> >>> - throw new
> >>> InvalidStateException(_loc.get("closed"), _closed).
> >>> - setFatal(true);
> >>> + if (_closed) {
> >>> + if (_closedException == null) // TRACE not enabled
> >>> + throw new
> >>> InvalidStateException(_loc.get("closed-notrace"))
> >>> + .setFatal(true);
> >>> + else
> >>> + throw new
> InvalidStateException(_loc.get("closed"),
> >>> + _closedException).setFatal(true);
> >>> + }
> >>> }
> >>>
> >>> public void assertActiveTransaction() {
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/FetchConfigurationImpl.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/java/org/apache/openjpa/kernel/
> >> FetchConfigurationImpl.jav
> >> a?> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/FetchConfigurationImpl.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/kernel/FetchConfigurationImpl.java Sun Feb 11
> 18:33:05 2007
> >>> @@ -36,6 +36,8 @@
> >>> import org.apache.openjpa.lib.rop.SimpleResultList;
> >>> import org.apache.openjpa.lib.rop.WindowResultList;
> >>> import org.apache.openjpa.lib.util.Localizer;
> >>> +import org.apache.openjpa.lib.util.ReferenceMap;
> >>> +import
> >>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> >>> import org.apache.openjpa.meta.ClassMetaData;
> >>> import org.apache.openjpa.meta.FetchGroup;
> >>> import org.apache.openjpa.meta.FieldMetaData;
> >>> @@ -58,6 +60,10 @@
> >>> private static final Localizer _loc = Localizer.forPackage
> >>> (FetchConfigurationImpl.class);
> >>>
> >>> + // Cache the from/to isAssignable invocations
> >>> + private static ConcurrentReferenceHashMap _assignableTypes =
> >>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> >>> ReferenceMap.WEAK);
> >>> +
> >>> /**
> >>> * Configurable state shared throughout a traversal chain.
> >>> */
> >>> @@ -613,11 +619,37 @@
> >>> }
> >>>
> >>> /**
> >>> - * Whether either of the two types is assignable from the
> >>> other.
> >>> + * Whether either of the two types is assignable from
> >>> the other. Optimize
> >>> + * for the repeat calls with similar parameters by
> >>> caching the from/to
> >>> + * type parameters.
> >>> */
> >>> - private static boolean isAssignable(Class c1, Class c2) {
> >>> - return c1 != null && c2 != null
> >>> - && (c1.isAssignableFrom(c2) || c2.isAssignableFrom
> >>> (c1));
> >>> + private static boolean isAssignable(Class from, Class to) {
> >>> + boolean isAssignable;
> >>> +
> >>> + if (from == null || to == null)
> >>> + return false;
> >>> + ConcurrentReferenceHashMap assignableTo =
> >>> + (ConcurrentReferenceHashMap) _assignableTypes.get
> >>> (from);
> >>> +
> >>> + if (assignableTo != null) { // "to" cache exists...
> >>> + isAssignable = (assignableTo.get(to) != null);
> >>> + if (!isAssignable) { // not in the map yet...
> >>> + isAssignable = from.isAssignableFrom(to);
> >>> + if (isAssignable) {
> >>> + assignableTo.put(to, new Object());
> >>> + }
> >>> + }
> >>> + } else { // no "to" cache yet...
> >>> + isAssignable = from.isAssignableFrom(to);
> >>> + if (isAssignable) {
> >>> + assignableTo = new ConcurrentReferenceHashMap(
> >>> + ReferenceMap.HARD, ReferenceMap.WEAK);
> >>> + _assignableTypes.put(from, assignableTo);
> >>> + assignableTo.put(to, new Object());
> >>> + }
> >>> + }
> >>> +
> >>> + return isAssignable;
> >>> }
> >>>
> >>> /**
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/util/OpenJPAId.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java?
> >> view=diff&rev
> >> => 506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/util/OpenJPAId.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> >> e/openjpa/util/OpenJPAId.java Sun Feb 11 18:33:05 2007
> >>> @@ -17,6 +17,10 @@
> >>>
> >>> import java.io.Serializable;
> >>>
> >>> +import org.apache.openjpa.lib.util.ReferenceHashSet;
> >>> +import org.apache.openjpa.lib.util.ReferenceMap;
> >>> +import
> >>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> >>> +
> >>> /**
> >>> * Identity class extended by builtin OpenJPA identity objects.
> >>> *
> >>> @@ -31,6 +35,9 @@
> >>> // type has his based on the least-derived non-object
> >>> class so that
> >>> // user-given ids with non-exact types match ids with exact
> >>> types
> >>> private transient int _typeHash = 0;
> >>> + // cache the types' generated hashcodes
> >>> + private static ConcurrentReferenceHashMap _typeCache =
> >>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> >>> ReferenceMap.WEAK);
> >>>
> >>> protected OpenJPAId() {
> >>> }
> >>> @@ -82,13 +89,25 @@
> >>> */
> >>> protected abstract boolean idEquals(OpenJPAId other);
> >>>
> >>> + /**
> >>> + * Generate the hashcode for this Id. Cache the type's
> >>> generated hashcode
> >>> + * so that it doesn't have to be generated each time.
> >>> + */
> >>> public int hashCode() {
> >>> if (_typeHash == 0) {
> >>> - Class base = type;
> >>> - while (base.getSuperclass() != null
> >>> - && base.getSuperclass() != Object.class)
> >>> - base = base.getSuperclass();
> >>> - _typeHash = base.hashCode();
> >>> + Integer typeHashInt = (Integer) _typeCache.get(type);
> >>> + if (typeHashInt == null) {
> >>> + Class base = type;
> >>> + Class superclass = base.getSuperclass();
> >>> + while (superclass != null && superclass !=
> >>> Object.class) {
> >>> + base = base.getSuperclass();
> >>> + superclass = base.getSuperclass();
> >>> + }
> >>> + _typeHash = base.hashCode();
> >>> + _typeCache.put(type, new Integer(_typeHash));
> >>> + } else {
> >>> + _typeHash = typeHashInt.intValue();
> >>> + }
> >>> }
> >>> return _typeHash ^ idHash();
> >>> }
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> >> apache/openjpa/kernel/localizer.properties
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> >> ernel/src/main/resources/org/apache/openjpa/kernel/
> >> localizer.properties?
> >>> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> >> apache/openjpa/kernel/localizer.properties (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> >> apache/openjpa/kernel/localizer.properties Sun Feb 11 18:33:05 2007
> >>> @@ -94,8 +94,13 @@
> >>> active: This operation cannot be performed while a
> >>> Transaction is active.
> >>> closed: The context has been closed. The stack trace at which
> >>> the \
> >>> context was closed is held in the embedded exception.
> >>> +closed-notrace: The context has been closed. The stack
> >>> trace at which the \
> >>> + context was closed is available if Runtime=TRACE
> >>> logging is enabled.
> >>> closed-factory: The factory has been closed. The stack
> trace at \
> >>> which the factory was closed is held in the embedded exception.
> >>> +closed-factory-notrace: The factory has been closed. The
> >>> stack trace at \
> >>> + which the factory was closed is available if
> >>> Runtime=TRACE logging is \
> >>> + enabled.
> >>> non-trans-read: To perform reads on persistent data outside
> >>> of a transaction, \
> >>> the "NontransactionalRead" property must be set on the
> >>> Transaction.
> >>> non-trans-write: To perform writes on persistent data
> outside of
> >>> a \
> >>>
> >>> Modified:
> >>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> >> penjpa/lib/conf/ObjectValue.java
> >>> URL:
> >>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-l
> >> ib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?>
> >> view=diff&rev=506230&r1=506229&r2=506230
> >>> ==============================================================
> >>> ================
> >>> ---
> >>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> >> penjpa/lib/conf/ObjectValue.java (original)
> >>> +++
> >>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> >> penjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
> >>> @@ -17,6 +17,8 @@
> >>>
> >>> import org.apache.commons.lang.ObjectUtils;
> >>> import org.apache.openjpa.lib.util.Localizer;
> >>> +import org.apache.openjpa.lib.util.ReferenceMap;
> >>> +import
> >>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> >>>
> >>> /**
> >>> * An object {@link Value}.
> >>> @@ -28,6 +30,10 @@
> >>> private static final Localizer _loc = Localizer.forPackage
> >>> (ObjectValue.class);
> >>>
> >>> + // cache the types' classloader
> >>> + private static ConcurrentReferenceHashMap _classloaderCache =
> >>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> >>> ReferenceMap.WEAK);
> >>> +
> >>> private Object _value = null;
> >>>
> >>> public ObjectValue(String prop) {
> >>> @@ -81,10 +87,16 @@
> >>> * Allow subclasses to instantiate additional plugins.
> >>> This method does
> >>> * not perform configuration.
> >>> */
> >>> - public Object newInstance(String clsName, Class type,
> >>> - Configuration conf, boolean fatal) {
> >>> - return Configurations.newInstance(clsName, this, conf,
> >>> - type.getClassLoader(), fatal);
> >>> + public Object newInstance(String clsName, Class type,
> >>> Configuration conf,
> >>> + boolean fatal) {
> >>> + ClassLoader cl = (ClassLoader)
> _classloaderCache.get(type);
> >>> + if (cl == null) {
> >>> + cl = type.getClassLoader();
> >>> + if (cl != null) { // System classloader is
> >>> returned as null
> >>> + _classloaderCache.put(type, cl);
> >>> + }
> >>> + }
> >>> + return Configurations.newInstance(clsName, this,
> >>> conf, cl, fatal);
> >>> }
> >>>
> >>> public Class getValueType() {
> >>>
> >>>
> >>>
> >
>
>
Re: svn commit: r506230 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/ee/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/util/ openjpa-kernel/src/main/resources/org/
Posted by Dain Sundstrom <da...@iq80.com>.
Not sure if this is relevant, but normally, I use explicit, thread
and then system.
-dain
On Feb 26, 2007, at 10:52 PM, Marc Prud'hommeaux wrote:
>
> How about just assigning cl = ClassLoader.getSystemClassLoader()
> (which will hopefully never be null) when cl is null?
>
>
>
> On Feb 27, 2007, at 7:29 AM, Patrick Linskey wrote:
>
>> Hi,
>>
>> I was just looking at ObjectValue, and noticed something odd in
>> the code
>> snippet below. It looks like if cl is null, we will end up with
>> constant
>> cache misses. So, outside a container, we'll always be doing the
>> extra
>> cache overhead, and will never get any benefit from doing so.
>>
>> Should we put some sort of placeholder in place for 'cl' when it's
>> null
>> to disambiguate?
>>
>> -Patrick
>>
>>> - public Object newInstance(String clsName, Class type,
>>> - Configuration conf, boolean fatal) {
>>> - return Configurations.newInstance(clsName, this, conf,
>>> - type.getClassLoader(), fatal);
>>> + public Object newInstance(String clsName, Class type,
>>> Configuration conf,
>>> + boolean fatal) {
>>> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
>>> + if (cl == null) {
>>> + cl = type.getClassLoader();
>>> + if (cl != null) { // System classloader is
>>> returned as null
>>> + _classloaderCache.put(type, cl);
>>> + }
>>> + }
>>> + return Configurations.newInstance(clsName, this,
>>> conf, cl, fatal);
>>> }
>>
>> --
>> Patrick Linskey
>> BEA Systems, Inc.
>>
>> _____________________________________________________________________
>> __
>> Notice: This email message, together with any attachments, may
>> contain
>> information of BEA Systems, Inc., its subsidiaries and
>> affiliated
>> entities, that may be confidential, proprietary, copyrighted
>> and/or
>> legally privileged, and is intended solely for the use of the
>> individual
>> or entity named in this message. If you are not the intended
>> recipient,
>> and have received this message in error, please immediately return
>> this
>> by email and then delete it.
>>
>>> -----Original Message-----
>>> From: kwsutter@apache.org [mailto:kwsutter@apache.org]
>>> Sent: Sunday, February 11, 2007 6:33 PM
>>> To: open-jpa-commits@incubator.apache.org
>>> Subject: svn commit: r506230 - in /incubator/openjpa/trunk:
>>> openjpa-kernel/src/main/java/org/apache/openjpa/ee/
>>> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
>>> openjpa-kernel/src/main/java/org/apache/openjpa/util/
>>> openjpa-kernel/src/main/resources/org/a...
>>>
>>> Author: kwsutter
>>> Date: Sun Feb 11 18:33:05 2007
>>> New Revision: 506230
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=506230
>>> Log:
>>> OPENJPA-138. Some updates to help with performance of
>>> OpenJPA in an application server environment. Details can be
>>> found in the OPENJPA-138 Issue.
>>>
>>> Modified:
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/ee/JNDIManagedRuntime.java
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/AbstractBrokerFactory.java
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/BrokerImpl.java
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/FetchConfigurationImpl.java
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/util/OpenJPAId.java
>>>
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
>> apache/openjpa/kernel/localizer.properties
>>>
>>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
>> penjpa/lib/conf/ObjectValue.java
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/ee/JNDIManagedRuntime.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java?>
>> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/ee/JNDIManagedRuntime.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
>>> @@ -29,6 +29,7 @@
>>> implements ManagedRuntime {
>>>
>>> private String _tmLoc = "java:/TransactionManager";
>>> + private static TransactionManager _tm;
>>>
>>> /**
>>> * Return the location of the {@link TransactionManager} in
>>> JNDI.
>>> @@ -44,13 +45,18 @@
>>> _tmLoc = name;
>>> }
>>>
>>> - public TransactionManager getTransactionManager()
>>> - throws Exception {
>>> - Context ctx = new InitialContext();
>>> - try {
>>> - return (TransactionManager) ctx.lookup(_tmLoc);
>>> - } finally {
>>> - ctx.close();
>>> + /**
>>> + * Return the cached TransactionManager instance.
>>> + */
>>> + public TransactionManager getTransactionManager() throws
>>> Exception {
>>> + if (_tm == null) {
>>> + Context ctx = new InitialContext();
>>> + try {
>>> + _tm = (TransactionManager) ctx.lookup(_tmLoc);
>>> + } finally {
>>> + ctx.close();
>>> + }
>>> }
>>> - }
>>> + return _tm;
>>> + }
>>> }
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/AbstractBrokerFactory.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/java/org/apache/openjpa/kernel/
>> AbstractBrokerFactory.java
>> ?> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/AbstractBrokerFactory.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/AbstractBrokerFactory.java Sun Feb 11 18:33:05 2007
>>> @@ -64,7 +64,8 @@
>>> // configuration
>>> private final OpenJPAConfiguration _conf;
>>> private transient boolean _readOnly = false;
>>> - private transient RuntimeException _closed = null;
>>> + private transient boolean _closed = false;
>>> + private transient RuntimeException _closedException = null;
>>> private Map _userObjects = null;
>>>
>>> // internal lock: spec forbids synchronization on this object
>>> @@ -267,7 +268,7 @@
>>> * Returns true if this broker factory is closed.
>>> */
>>> public boolean isClosed() {
>>> - return _closed != null;
>>> + return _closed;
>>> }
>>>
>>> public void close() {
>>> @@ -297,7 +298,10 @@
>>> (_conf.getMetaDataRepositoryInstance());
>>>
>>> _conf.close();
>>> - _closed = new IllegalStateException();
>>> + _closed = true;
>>> + Log log = _conf.getLog
>>> (OpenJPAConfiguration.LOG_RUNTIME);
>>> + if (log.isTraceEnabled())
>>> + _closedException = new IllegalStateException();
>>> } finally {
>>> unlock();
>>> }
>>> @@ -546,12 +550,18 @@
>>> }
>>>
>>> /**
>>> - * Throw an exception if the factory is closed.
>>> + * Throw an exception if the factory is closed. The
>>> exact message and
>>> + * content of the exception varies whether TRACE is
>>> enabled or not.
>>> */
>>> private void assertOpen() {
>>> - if (_closed != null)
>>> - throw new
>>> InvalidStateException(_loc.get("closed-factory")).
>>> - setCause(_closed);
>>> + if (_closed) {
>>> + if (_closedException == null) // TRACE not enabled
>>> + throw new InvalidStateException(_loc
>>> + .get("closed-factory-notrace"));
>>> + else
>>> + throw new
>>> InvalidStateException(_loc.get("closed-factory"))
>>> + .setCause(_closedException);
>>> + }
>>> }
>>>
>>> ////////////////////
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/BrokerImpl.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?>
>> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/BrokerImpl.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/BrokerImpl.java Sun Feb 11 18:33:05 2007
>>> @@ -63,6 +63,7 @@
>>> import org.apache.openjpa.lib.util.ReferenceHashMap;
>>> import org.apache.openjpa.lib.util.ReferenceHashSet;
>>> import org.apache.openjpa.lib.util.ReferenceMap;
>>> +import
>>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>>> import org.apache.openjpa.lib.util.concurrent.ReentrantLock;
>>> import org.apache.openjpa.meta.ClassMetaData;
>>> import org.apache.openjpa.meta.FieldMetaData;
>>> @@ -138,6 +139,9 @@
>>>
>>> private static final Localizer _loc =
>>> Localizer.forPackage(BrokerImpl.class);
>>> + // Cache for from/to type assignments
>>> + private static ConcurrentReferenceHashMap _assignableTypes =
>>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>>> ReferenceMap.WEAK);
>>>
>>> // the store manager in use; this may be a
>>> decorator such as a
>>> // data cache store manager around the native store manager
>>> @@ -215,7 +219,8 @@
>>>
>>> // status
>>> private int _flags = 0;
>>> - private RuntimeException _closed = null;
>>> + private boolean _closed = false;
>>> + private RuntimeException _closedException = null;
>>>
>>> // event managers
>>> private TransactionEventManager _transEventManager = null;
>>> @@ -1096,8 +1101,7 @@
>>> cls));
>>> return PCRegistry.newObjectId(cls, (String) val);
>>> }
>>> -
>>> - if
>>> (meta.getObjectIdType().isAssignableFrom(val.getClass())) {
>>> + if (isAssignable(meta.getObjectIdType(),
>>> val.getClass())) {
>>> if (!meta.isOpenJPAIdentity() &&
>>> meta.isObjectIdTypeShared())
>>> return new ObjectId(cls, val);
>>> return val;
>>> @@ -1119,6 +1123,37 @@
>>> }
>>>
>>> /**
>>> + * Cache from/to assignments to avoid
>>> Class.isAssignableFrom overhead
>>> + * @param from the target Class
>>> + * @param to the Class to test
>>> + * @return true if the "to" class could be assigned to
>>> "from" class
>>> + */
>>> + private boolean isAssignable(Class from, Class to) {
>>> + boolean isAssignable;
>>> + ConcurrentReferenceHashMap assignableTo =
>>> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
>>> +
>>> + if (assignableTo != null) { // "to" cache exists...
>>> + isAssignable = (assignableTo.get(to) != null);
>>> + if (!isAssignable) { // not in the map yet...
>>> + isAssignable = from.isAssignableFrom(to);
>>> + if (isAssignable) {
>>> + assignableTo.put(to, new Object());
>>> + }
>>> + }
>>> + } else { // no "to" cache yet...
>>> + isAssignable = from.isAssignableFrom(to);
>>> + if (isAssignable) {
>>> + assignableTo = new ConcurrentReferenceHashMap(
>>> + ReferenceMap.HARD, ReferenceMap.WEAK);
>>> + _assignableTypes.put(from, assignableTo);
>>> + assignableTo.put(to, new Object());
>>> + }
>>> + }
>>> + return isAssignable;
>>> + }
>>> +
>>> + /**
>>> * Create a new state manager for the given oid.
>>> */
>>> private StateManagerImpl newStateManagerImpl(Object oid,
>>> boolean copy) {
>>> @@ -3969,11 +4004,11 @@
>>> ///////////
>>>
>>> public boolean isClosed() {
>>> - return _closed != null;
>>> + return _closed;
>>> }
>>>
>>> public boolean isCloseInvoked() {
>>> - return _closed != null || (_flags & FLAG_CLOSE_INVOKED) !
>>> = 0;
>>> + return _closed || (_flags & FLAG_CLOSE_INVOKED) != 0;
>>> }
>>>
>>> public void close() {
>>> @@ -4055,8 +4090,10 @@
>>>
>>> _lm.close();
>>> _store.close();
>>> - _closed = new IllegalStateException();
>>> _flags = 0;
>>> + _closed = true;
>>> + if (_log.isTraceEnabled())
>>> + _closedException = new IllegalStateException();
>>>
>>> if (err != null)
>>> throw err;
>>> @@ -4246,11 +4283,19 @@
>>> /////////
>>> // Utils
>>> /////////
>>> -
>>> + /**
>>> + * Throw an exception if the context is closed. The
>>> exact message and
>>> + * content of the exception varies whether TRACE is
>>> enabled or not.
>>> + */
>>> public void assertOpen() {
>>> - if (_closed != null)
>>> - throw new
>>> InvalidStateException(_loc.get("closed"), _closed).
>>> - setFatal(true);
>>> + if (_closed) {
>>> + if (_closedException == null) // TRACE not enabled
>>> + throw new
>>> InvalidStateException(_loc.get("closed-notrace"))
>>> + .setFatal(true);
>>> + else
>>> + throw new InvalidStateException(_loc.get("closed"),
>>> + _closedException).setFatal(true);
>>> + }
>>> }
>>>
>>> public void assertActiveTransaction() {
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/FetchConfigurationImpl.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/java/org/apache/openjpa/kernel/
>> FetchConfigurationImpl.jav
>> a?> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/FetchConfigurationImpl.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/kernel/FetchConfigurationImpl.java Sun Feb 11 18:33:05 2007
>>> @@ -36,6 +36,8 @@
>>> import org.apache.openjpa.lib.rop.SimpleResultList;
>>> import org.apache.openjpa.lib.rop.WindowResultList;
>>> import org.apache.openjpa.lib.util.Localizer;
>>> +import org.apache.openjpa.lib.util.ReferenceMap;
>>> +import
>>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>>> import org.apache.openjpa.meta.ClassMetaData;
>>> import org.apache.openjpa.meta.FetchGroup;
>>> import org.apache.openjpa.meta.FieldMetaData;
>>> @@ -58,6 +60,10 @@
>>> private static final Localizer _loc = Localizer.forPackage
>>> (FetchConfigurationImpl.class);
>>>
>>> + // Cache the from/to isAssignable invocations
>>> + private static ConcurrentReferenceHashMap _assignableTypes =
>>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>>> ReferenceMap.WEAK);
>>> +
>>> /**
>>> * Configurable state shared throughout a traversal chain.
>>> */
>>> @@ -613,11 +619,37 @@
>>> }
>>>
>>> /**
>>> - * Whether either of the two types is assignable from the
>>> other.
>>> + * Whether either of the two types is assignable from
>>> the other. Optimize
>>> + * for the repeat calls with similar parameters by
>>> caching the from/to
>>> + * type parameters.
>>> */
>>> - private static boolean isAssignable(Class c1, Class c2) {
>>> - return c1 != null && c2 != null
>>> - && (c1.isAssignableFrom(c2) || c2.isAssignableFrom
>>> (c1));
>>> + private static boolean isAssignable(Class from, Class to) {
>>> + boolean isAssignable;
>>> +
>>> + if (from == null || to == null)
>>> + return false;
>>> + ConcurrentReferenceHashMap assignableTo =
>>> + (ConcurrentReferenceHashMap) _assignableTypes.get
>>> (from);
>>> +
>>> + if (assignableTo != null) { // "to" cache exists...
>>> + isAssignable = (assignableTo.get(to) != null);
>>> + if (!isAssignable) { // not in the map yet...
>>> + isAssignable = from.isAssignableFrom(to);
>>> + if (isAssignable) {
>>> + assignableTo.put(to, new Object());
>>> + }
>>> + }
>>> + } else { // no "to" cache yet...
>>> + isAssignable = from.isAssignableFrom(to);
>>> + if (isAssignable) {
>>> + assignableTo = new ConcurrentReferenceHashMap(
>>> + ReferenceMap.HARD, ReferenceMap.WEAK);
>>> + _assignableTypes.put(from, assignableTo);
>>> + assignableTo.put(to, new Object());
>>> + }
>>> + }
>>> +
>>> + return isAssignable;
>>> }
>>>
>>> /**
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/util/OpenJPAId.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java?
>> view=diff&rev
>> => 506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/util/OpenJPAId.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
>> e/openjpa/util/OpenJPAId.java Sun Feb 11 18:33:05 2007
>>> @@ -17,6 +17,10 @@
>>>
>>> import java.io.Serializable;
>>>
>>> +import org.apache.openjpa.lib.util.ReferenceHashSet;
>>> +import org.apache.openjpa.lib.util.ReferenceMap;
>>> +import
>>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>>> +
>>> /**
>>> * Identity class extended by builtin OpenJPA identity objects.
>>> *
>>> @@ -31,6 +35,9 @@
>>> // type has his based on the least-derived non-object
>>> class so that
>>> // user-given ids with non-exact types match ids with exact
>>> types
>>> private transient int _typeHash = 0;
>>> + // cache the types' generated hashcodes
>>> + private static ConcurrentReferenceHashMap _typeCache =
>>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>>> ReferenceMap.WEAK);
>>>
>>> protected OpenJPAId() {
>>> }
>>> @@ -82,13 +89,25 @@
>>> */
>>> protected abstract boolean idEquals(OpenJPAId other);
>>>
>>> + /**
>>> + * Generate the hashcode for this Id. Cache the type's
>>> generated hashcode
>>> + * so that it doesn't have to be generated each time.
>>> + */
>>> public int hashCode() {
>>> if (_typeHash == 0) {
>>> - Class base = type;
>>> - while (base.getSuperclass() != null
>>> - && base.getSuperclass() != Object.class)
>>> - base = base.getSuperclass();
>>> - _typeHash = base.hashCode();
>>> + Integer typeHashInt = (Integer) _typeCache.get(type);
>>> + if (typeHashInt == null) {
>>> + Class base = type;
>>> + Class superclass = base.getSuperclass();
>>> + while (superclass != null && superclass !=
>>> Object.class) {
>>> + base = base.getSuperclass();
>>> + superclass = base.getSuperclass();
>>> + }
>>> + _typeHash = base.hashCode();
>>> + _typeCache.put(type, new Integer(_typeHash));
>>> + } else {
>>> + _typeHash = typeHashInt.intValue();
>>> + }
>>> }
>>> return _typeHash ^ idHash();
>>> }
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
>> apache/openjpa/kernel/localizer.properties
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
>> ernel/src/main/resources/org/apache/openjpa/kernel/
>> localizer.properties?
>>> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
>> apache/openjpa/kernel/localizer.properties (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
>> apache/openjpa/kernel/localizer.properties Sun Feb 11 18:33:05 2007
>>> @@ -94,8 +94,13 @@
>>> active: This operation cannot be performed while a
>>> Transaction is active.
>>> closed: The context has been closed. The stack trace at which
>>> the \
>>> context was closed is held in the embedded exception.
>>> +closed-notrace: The context has been closed. The stack
>>> trace at which the \
>>> + context was closed is available if Runtime=TRACE
>>> logging is enabled.
>>> closed-factory: The factory has been closed. The stack trace at \
>>> which the factory was closed is held in the embedded exception.
>>> +closed-factory-notrace: The factory has been closed. The
>>> stack trace at \
>>> + which the factory was closed is available if
>>> Runtime=TRACE logging is \
>>> + enabled.
>>> non-trans-read: To perform reads on persistent data outside
>>> of a transaction, \
>>> the "NontransactionalRead" property must be set on the
>>> Transaction.
>>> non-trans-write: To perform writes on persistent data outside of
>>> a \
>>>
>>> Modified:
>>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
>> penjpa/lib/conf/ObjectValue.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-l
>> ib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?>
>> view=diff&rev=506230&r1=506229&r2=506230
>>> ==============================================================
>>> ================
>>> ---
>>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
>> penjpa/lib/conf/ObjectValue.java (original)
>>> +++
>>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
>> penjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
>>> @@ -17,6 +17,8 @@
>>>
>>> import org.apache.commons.lang.ObjectUtils;
>>> import org.apache.openjpa.lib.util.Localizer;
>>> +import org.apache.openjpa.lib.util.ReferenceMap;
>>> +import
>>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>>>
>>> /**
>>> * An object {@link Value}.
>>> @@ -28,6 +30,10 @@
>>> private static final Localizer _loc = Localizer.forPackage
>>> (ObjectValue.class);
>>>
>>> + // cache the types' classloader
>>> + private static ConcurrentReferenceHashMap _classloaderCache =
>>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>>> ReferenceMap.WEAK);
>>> +
>>> private Object _value = null;
>>>
>>> public ObjectValue(String prop) {
>>> @@ -81,10 +87,16 @@
>>> * Allow subclasses to instantiate additional plugins.
>>> This method does
>>> * not perform configuration.
>>> */
>>> - public Object newInstance(String clsName, Class type,
>>> - Configuration conf, boolean fatal) {
>>> - return Configurations.newInstance(clsName, this, conf,
>>> - type.getClassLoader(), fatal);
>>> + public Object newInstance(String clsName, Class type,
>>> Configuration conf,
>>> + boolean fatal) {
>>> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
>>> + if (cl == null) {
>>> + cl = type.getClassLoader();
>>> + if (cl != null) { // System classloader is
>>> returned as null
>>> + _classloaderCache.put(type, cl);
>>> + }
>>> + }
>>> + return Configurations.newInstance(clsName, this,
>>> conf, cl, fatal);
>>> }
>>>
>>> public Class getValueType() {
>>>
>>>
>>>
>
Re: svn commit: r506230 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/ee/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/util/ openjpa-kernel/src/main/resources/org/
Posted by Marc Prud'hommeaux <mp...@apache.org>.
How about just assigning cl = ClassLoader.getSystemClassLoader()
(which will hopefully never be null) when cl is null?
On Feb 27, 2007, at 7:29 AM, Patrick Linskey wrote:
> Hi,
>
> I was just looking at ObjectValue, and noticed something odd in the
> code
> snippet below. It looks like if cl is null, we will end up with
> constant
> cache misses. So, outside a container, we'll always be doing the extra
> cache overhead, and will never get any benefit from doing so.
>
> Should we put some sort of placeholder in place for 'cl' when it's
> null
> to disambiguate?
>
> -Patrick
>
>> - public Object newInstance(String clsName, Class type,
>> - Configuration conf, boolean fatal) {
>> - return Configurations.newInstance(clsName, this, conf,
>> - type.getClassLoader(), fatal);
>> + public Object newInstance(String clsName, Class type,
>> Configuration conf,
>> + boolean fatal) {
>> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
>> + if (cl == null) {
>> + cl = type.getClassLoader();
>> + if (cl != null) { // System classloader is
>> returned as null
>> + _classloaderCache.put(type, cl);
>> + }
>> + }
>> + return Configurations.newInstance(clsName, this,
>> conf, cl, fatal);
>> }
>
> --
> Patrick Linskey
> BEA Systems, Inc.
>
> ______________________________________________________________________
> _
> Notice: This email message, together with any attachments, may
> contain
> information of BEA Systems, Inc., its subsidiaries and
> affiliated
> entities, that may be confidential, proprietary, copyrighted
> and/or
> legally privileged, and is intended solely for the use of the
> individual
> or entity named in this message. If you are not the intended
> recipient,
> and have received this message in error, please immediately return
> this
> by email and then delete it.
>
>> -----Original Message-----
>> From: kwsutter@apache.org [mailto:kwsutter@apache.org]
>> Sent: Sunday, February 11, 2007 6:33 PM
>> To: open-jpa-commits@incubator.apache.org
>> Subject: svn commit: r506230 - in /incubator/openjpa/trunk:
>> openjpa-kernel/src/main/java/org/apache/openjpa/ee/
>> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
>> openjpa-kernel/src/main/java/org/apache/openjpa/util/
>> openjpa-kernel/src/main/resources/org/a...
>>
>> Author: kwsutter
>> Date: Sun Feb 11 18:33:05 2007
>> New Revision: 506230
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=506230
>> Log:
>> OPENJPA-138. Some updates to help with performance of
>> OpenJPA in an application server environment. Details can be
>> found in the OPENJPA-138 Issue.
>>
>> Modified:
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/ee/JNDIManagedRuntime.java
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/AbstractBrokerFactory.java
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/BrokerImpl.java
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/FetchConfigurationImpl.java
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/util/OpenJPAId.java
>>
>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> apache/openjpa/kernel/localizer.properties
>>
>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> penjpa/lib/conf/ObjectValue.java
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/ee/JNDIManagedRuntime.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java?>
> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/ee/JNDIManagedRuntime.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
>> @@ -29,6 +29,7 @@
>> implements ManagedRuntime {
>>
>> private String _tmLoc = "java:/TransactionManager";
>> + private static TransactionManager _tm;
>>
>> /**
>> * Return the location of the {@link TransactionManager} in
>> JNDI.
>> @@ -44,13 +45,18 @@
>> _tmLoc = name;
>> }
>>
>> - public TransactionManager getTransactionManager()
>> - throws Exception {
>> - Context ctx = new InitialContext();
>> - try {
>> - return (TransactionManager) ctx.lookup(_tmLoc);
>> - } finally {
>> - ctx.close();
>> + /**
>> + * Return the cached TransactionManager instance.
>> + */
>> + public TransactionManager getTransactionManager() throws
>> Exception {
>> + if (_tm == null) {
>> + Context ctx = new InitialContext();
>> + try {
>> + _tm = (TransactionManager) ctx.lookup(_tmLoc);
>> + } finally {
>> + ctx.close();
>> + }
>> }
>> - }
>> + return _tm;
>> + }
>> }
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/AbstractBrokerFactory.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/java/org/apache/openjpa/kernel/
> AbstractBrokerFactory.java
> ?> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/AbstractBrokerFactory.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/AbstractBrokerFactory.java Sun Feb 11 18:33:05 2007
>> @@ -64,7 +64,8 @@
>> // configuration
>> private final OpenJPAConfiguration _conf;
>> private transient boolean _readOnly = false;
>> - private transient RuntimeException _closed = null;
>> + private transient boolean _closed = false;
>> + private transient RuntimeException _closedException = null;
>> private Map _userObjects = null;
>>
>> // internal lock: spec forbids synchronization on this object
>> @@ -267,7 +268,7 @@
>> * Returns true if this broker factory is closed.
>> */
>> public boolean isClosed() {
>> - return _closed != null;
>> + return _closed;
>> }
>>
>> public void close() {
>> @@ -297,7 +298,10 @@
>> (_conf.getMetaDataRepositoryInstance());
>>
>> _conf.close();
>> - _closed = new IllegalStateException();
>> + _closed = true;
>> + Log log = _conf.getLog
>> (OpenJPAConfiguration.LOG_RUNTIME);
>> + if (log.isTraceEnabled())
>> + _closedException = new IllegalStateException();
>> } finally {
>> unlock();
>> }
>> @@ -546,12 +550,18 @@
>> }
>>
>> /**
>> - * Throw an exception if the factory is closed.
>> + * Throw an exception if the factory is closed. The
>> exact message and
>> + * content of the exception varies whether TRACE is
>> enabled or not.
>> */
>> private void assertOpen() {
>> - if (_closed != null)
>> - throw new
>> InvalidStateException(_loc.get("closed-factory")).
>> - setCause(_closed);
>> + if (_closed) {
>> + if (_closedException == null) // TRACE not enabled
>> + throw new InvalidStateException(_loc
>> + .get("closed-factory-notrace"));
>> + else
>> + throw new
>> InvalidStateException(_loc.get("closed-factory"))
>> + .setCause(_closedException);
>> + }
>> }
>>
>> ////////////////////
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/BrokerImpl.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?>
> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/BrokerImpl.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/BrokerImpl.java Sun Feb 11 18:33:05 2007
>> @@ -63,6 +63,7 @@
>> import org.apache.openjpa.lib.util.ReferenceHashMap;
>> import org.apache.openjpa.lib.util.ReferenceHashSet;
>> import org.apache.openjpa.lib.util.ReferenceMap;
>> +import
>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>> import org.apache.openjpa.lib.util.concurrent.ReentrantLock;
>> import org.apache.openjpa.meta.ClassMetaData;
>> import org.apache.openjpa.meta.FieldMetaData;
>> @@ -138,6 +139,9 @@
>>
>> private static final Localizer _loc =
>> Localizer.forPackage(BrokerImpl.class);
>> + // Cache for from/to type assignments
>> + private static ConcurrentReferenceHashMap _assignableTypes =
>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>> ReferenceMap.WEAK);
>>
>> // the store manager in use; this may be a
>> decorator such as a
>> // data cache store manager around the native store manager
>> @@ -215,7 +219,8 @@
>>
>> // status
>> private int _flags = 0;
>> - private RuntimeException _closed = null;
>> + private boolean _closed = false;
>> + private RuntimeException _closedException = null;
>>
>> // event managers
>> private TransactionEventManager _transEventManager = null;
>> @@ -1096,8 +1101,7 @@
>> cls));
>> return PCRegistry.newObjectId(cls, (String) val);
>> }
>> -
>> - if
>> (meta.getObjectIdType().isAssignableFrom(val.getClass())) {
>> + if (isAssignable(meta.getObjectIdType(),
>> val.getClass())) {
>> if (!meta.isOpenJPAIdentity() &&
>> meta.isObjectIdTypeShared())
>> return new ObjectId(cls, val);
>> return val;
>> @@ -1119,6 +1123,37 @@
>> }
>>
>> /**
>> + * Cache from/to assignments to avoid
>> Class.isAssignableFrom overhead
>> + * @param from the target Class
>> + * @param to the Class to test
>> + * @return true if the "to" class could be assigned to
>> "from" class
>> + */
>> + private boolean isAssignable(Class from, Class to) {
>> + boolean isAssignable;
>> + ConcurrentReferenceHashMap assignableTo =
>> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
>> +
>> + if (assignableTo != null) { // "to" cache exists...
>> + isAssignable = (assignableTo.get(to) != null);
>> + if (!isAssignable) { // not in the map yet...
>> + isAssignable = from.isAssignableFrom(to);
>> + if (isAssignable) {
>> + assignableTo.put(to, new Object());
>> + }
>> + }
>> + } else { // no "to" cache yet...
>> + isAssignable = from.isAssignableFrom(to);
>> + if (isAssignable) {
>> + assignableTo = new ConcurrentReferenceHashMap(
>> + ReferenceMap.HARD, ReferenceMap.WEAK);
>> + _assignableTypes.put(from, assignableTo);
>> + assignableTo.put(to, new Object());
>> + }
>> + }
>> + return isAssignable;
>> + }
>> +
>> + /**
>> * Create a new state manager for the given oid.
>> */
>> private StateManagerImpl newStateManagerImpl(Object oid,
>> boolean copy) {
>> @@ -3969,11 +4004,11 @@
>> ///////////
>>
>> public boolean isClosed() {
>> - return _closed != null;
>> + return _closed;
>> }
>>
>> public boolean isCloseInvoked() {
>> - return _closed != null || (_flags & FLAG_CLOSE_INVOKED) !
>> = 0;
>> + return _closed || (_flags & FLAG_CLOSE_INVOKED) != 0;
>> }
>>
>> public void close() {
>> @@ -4055,8 +4090,10 @@
>>
>> _lm.close();
>> _store.close();
>> - _closed = new IllegalStateException();
>> _flags = 0;
>> + _closed = true;
>> + if (_log.isTraceEnabled())
>> + _closedException = new IllegalStateException();
>>
>> if (err != null)
>> throw err;
>> @@ -4246,11 +4283,19 @@
>> /////////
>> // Utils
>> /////////
>> -
>> + /**
>> + * Throw an exception if the context is closed. The
>> exact message and
>> + * content of the exception varies whether TRACE is
>> enabled or not.
>> + */
>> public void assertOpen() {
>> - if (_closed != null)
>> - throw new
>> InvalidStateException(_loc.get("closed"), _closed).
>> - setFatal(true);
>> + if (_closed) {
>> + if (_closedException == null) // TRACE not enabled
>> + throw new
>> InvalidStateException(_loc.get("closed-notrace"))
>> + .setFatal(true);
>> + else
>> + throw new InvalidStateException(_loc.get("closed"),
>> + _closedException).setFatal(true);
>> + }
>> }
>>
>> public void assertActiveTransaction() {
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/FetchConfigurationImpl.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/java/org/apache/openjpa/kernel/
> FetchConfigurationImpl.jav
> a?> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/FetchConfigurationImpl.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/kernel/FetchConfigurationImpl.java Sun Feb 11 18:33:05 2007
>> @@ -36,6 +36,8 @@
>> import org.apache.openjpa.lib.rop.SimpleResultList;
>> import org.apache.openjpa.lib.rop.WindowResultList;
>> import org.apache.openjpa.lib.util.Localizer;
>> +import org.apache.openjpa.lib.util.ReferenceMap;
>> +import
>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>> import org.apache.openjpa.meta.ClassMetaData;
>> import org.apache.openjpa.meta.FetchGroup;
>> import org.apache.openjpa.meta.FieldMetaData;
>> @@ -58,6 +60,10 @@
>> private static final Localizer _loc = Localizer.forPackage
>> (FetchConfigurationImpl.class);
>>
>> + // Cache the from/to isAssignable invocations
>> + private static ConcurrentReferenceHashMap _assignableTypes =
>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>> ReferenceMap.WEAK);
>> +
>> /**
>> * Configurable state shared throughout a traversal chain.
>> */
>> @@ -613,11 +619,37 @@
>> }
>>
>> /**
>> - * Whether either of the two types is assignable from the other.
>> + * Whether either of the two types is assignable from
>> the other. Optimize
>> + * for the repeat calls with similar parameters by
>> caching the from/to
>> + * type parameters.
>> */
>> - private static boolean isAssignable(Class c1, Class c2) {
>> - return c1 != null && c2 != null
>> - && (c1.isAssignableFrom(c2) || c2.isAssignableFrom(c1));
>> + private static boolean isAssignable(Class from, Class to) {
>> + boolean isAssignable;
>> +
>> + if (from == null || to == null)
>> + return false;
>> + ConcurrentReferenceHashMap assignableTo =
>> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
>> +
>> + if (assignableTo != null) { // "to" cache exists...
>> + isAssignable = (assignableTo.get(to) != null);
>> + if (!isAssignable) { // not in the map yet...
>> + isAssignable = from.isAssignableFrom(to);
>> + if (isAssignable) {
>> + assignableTo.put(to, new Object());
>> + }
>> + }
>> + } else { // no "to" cache yet...
>> + isAssignable = from.isAssignableFrom(to);
>> + if (isAssignable) {
>> + assignableTo = new ConcurrentReferenceHashMap(
>> + ReferenceMap.HARD, ReferenceMap.WEAK);
>> + _assignableTypes.put(from, assignableTo);
>> + assignableTo.put(to, new Object());
>> + }
>> + }
>> +
>> + return isAssignable;
>> }
>>
>> /**
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/util/OpenJPAId.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java?
> view=diff&rev
> => 506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/util/OpenJPAId.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
> e/openjpa/util/OpenJPAId.java Sun Feb 11 18:33:05 2007
>> @@ -17,6 +17,10 @@
>>
>> import java.io.Serializable;
>>
>> +import org.apache.openjpa.lib.util.ReferenceHashSet;
>> +import org.apache.openjpa.lib.util.ReferenceMap;
>> +import
>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>> +
>> /**
>> * Identity class extended by builtin OpenJPA identity objects.
>> *
>> @@ -31,6 +35,9 @@
>> // type has his based on the least-derived non-object
>> class so that
>> // user-given ids with non-exact types match ids with exact
>> types
>> private transient int _typeHash = 0;
>> + // cache the types' generated hashcodes
>> + private static ConcurrentReferenceHashMap _typeCache =
>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>> ReferenceMap.WEAK);
>>
>> protected OpenJPAId() {
>> }
>> @@ -82,13 +89,25 @@
>> */
>> protected abstract boolean idEquals(OpenJPAId other);
>>
>> + /**
>> + * Generate the hashcode for this Id. Cache the type's
>> generated hashcode
>> + * so that it doesn't have to be generated each time.
>> + */
>> public int hashCode() {
>> if (_typeHash == 0) {
>> - Class base = type;
>> - while (base.getSuperclass() != null
>> - && base.getSuperclass() != Object.class)
>> - base = base.getSuperclass();
>> - _typeHash = base.hashCode();
>> + Integer typeHashInt = (Integer) _typeCache.get(type);
>> + if (typeHashInt == null) {
>> + Class base = type;
>> + Class superclass = base.getSuperclass();
>> + while (superclass != null && superclass !=
>> Object.class) {
>> + base = base.getSuperclass();
>> + superclass = base.getSuperclass();
>> + }
>> + _typeHash = base.hashCode();
>> + _typeCache.put(type, new Integer(_typeHash));
>> + } else {
>> + _typeHash = typeHashInt.intValue();
>> + }
>> }
>> return _typeHash ^ idHash();
>> }
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> apache/openjpa/kernel/localizer.properties
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
> ernel/src/main/resources/org/apache/openjpa/kernel/
> localizer.properties?
>> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> apache/openjpa/kernel/localizer.properties (original)
>> +++
>> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
> apache/openjpa/kernel/localizer.properties Sun Feb 11 18:33:05 2007
>> @@ -94,8 +94,13 @@
>> active: This operation cannot be performed while a
>> Transaction is active.
>> closed: The context has been closed. The stack trace at which the \
>> context was closed is held in the embedded exception.
>> +closed-notrace: The context has been closed. The stack
>> trace at which the \
>> + context was closed is available if Runtime=TRACE
>> logging is enabled.
>> closed-factory: The factory has been closed. The stack trace at \
>> which the factory was closed is held in the embedded exception.
>> +closed-factory-notrace: The factory has been closed. The
>> stack trace at \
>> + which the factory was closed is available if
>> Runtime=TRACE logging is \
>> + enabled.
>> non-trans-read: To perform reads on persistent data outside
>> of a transaction, \
>> the "NontransactionalRead" property must be set on the
>> Transaction.
>> non-trans-write: To perform writes on persistent data outside of a \
>>
>> Modified:
>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> penjpa/lib/conf/ObjectValue.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-l
> ib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?>
> view=diff&rev=506230&r1=506229&r2=506230
>> ==============================================================
>> ================
>> ---
>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> penjpa/lib/conf/ObjectValue.java (original)
>> +++
>> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
> penjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
>> @@ -17,6 +17,8 @@
>>
>> import org.apache.commons.lang.ObjectUtils;
>> import org.apache.openjpa.lib.util.Localizer;
>> +import org.apache.openjpa.lib.util.ReferenceMap;
>> +import
>> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>>
>> /**
>> * An object {@link Value}.
>> @@ -28,6 +30,10 @@
>> private static final Localizer _loc = Localizer.forPackage
>> (ObjectValue.class);
>>
>> + // cache the types' classloader
>> + private static ConcurrentReferenceHashMap _classloaderCache =
>> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
>> ReferenceMap.WEAK);
>> +
>> private Object _value = null;
>>
>> public ObjectValue(String prop) {
>> @@ -81,10 +87,16 @@
>> * Allow subclasses to instantiate additional plugins.
>> This method does
>> * not perform configuration.
>> */
>> - public Object newInstance(String clsName, Class type,
>> - Configuration conf, boolean fatal) {
>> - return Configurations.newInstance(clsName, this, conf,
>> - type.getClassLoader(), fatal);
>> + public Object newInstance(String clsName, Class type,
>> Configuration conf,
>> + boolean fatal) {
>> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
>> + if (cl == null) {
>> + cl = type.getClassLoader();
>> + if (cl != null) { // System classloader is
>> returned as null
>> + _classloaderCache.put(type, cl);
>> + }
>> + }
>> + return Configurations.newInstance(clsName, this,
>> conf, cl, fatal);
>> }
>>
>> public Class getValueType() {
>>
>>
>>
RE: svn commit: r506230 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/ee/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/util/ openjpa-kernel/src/main/resources/org/
Posted by Patrick Linskey <pl...@bea.com>.
Hi,
I was just looking at ObjectValue, and noticed something odd in the code
snippet below. It looks like if cl is null, we will end up with constant
cache misses. So, outside a container, we'll always be doing the extra
cache overhead, and will never get any benefit from doing so.
Should we put some sort of placeholder in place for 'cl' when it's null
to disambiguate?
-Patrick
> - public Object newInstance(String clsName, Class type,
> - Configuration conf, boolean fatal) {
> - return Configurations.newInstance(clsName, this, conf,
> - type.getClassLoader(), fatal);
> + public Object newInstance(String clsName, Class type,
> Configuration conf,
> + boolean fatal) {
> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
> + if (cl == null) {
> + cl = type.getClassLoader();
> + if (cl != null) { // System classloader is
> returned as null
> + _classloaderCache.put(type, cl);
> + }
> + }
> + return Configurations.newInstance(clsName, this,
> conf, cl, fatal);
> }
--
Patrick Linskey
BEA Systems, Inc.
_______________________________________________________________________
Notice: This email message, together with any attachments, may contain
information of BEA Systems, Inc., its subsidiaries and affiliated
entities, that may be confidential, proprietary, copyrighted and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.
> -----Original Message-----
> From: kwsutter@apache.org [mailto:kwsutter@apache.org]
> Sent: Sunday, February 11, 2007 6:33 PM
> To: open-jpa-commits@incubator.apache.org
> Subject: svn commit: r506230 - in /incubator/openjpa/trunk:
> openjpa-kernel/src/main/java/org/apache/openjpa/ee/
> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
> openjpa-kernel/src/main/java/org/apache/openjpa/util/
> openjpa-kernel/src/main/resources/org/a...
>
> Author: kwsutter
> Date: Sun Feb 11 18:33:05 2007
> New Revision: 506230
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=506230
> Log:
> OPENJPA-138. Some updates to help with performance of
> OpenJPA in an application server environment. Details can be
> found in the OPENJPA-138 Issue.
>
> Modified:
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/ee/JNDIManagedRuntime.java
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/AbstractBrokerFactory.java
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/FetchConfigurationImpl.java
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/util/OpenJPAId.java
>
> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
apache/openjpa/kernel/localizer.properties
>
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
penjpa/lib/conf/ObjectValue.java
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/ee/JNDIManagedRuntime.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/ee/JNDIManagedRuntime.java?>
view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/ee/JNDIManagedRuntime.java (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
> @@ -29,6 +29,7 @@
> implements ManagedRuntime {
>
> private String _tmLoc = "java:/TransactionManager";
> + private static TransactionManager _tm;
>
> /**
> * Return the location of the {@link TransactionManager} in JNDI.
> @@ -44,13 +45,18 @@
> _tmLoc = name;
> }
>
> - public TransactionManager getTransactionManager()
> - throws Exception {
> - Context ctx = new InitialContext();
> - try {
> - return (TransactionManager) ctx.lookup(_tmLoc);
> - } finally {
> - ctx.close();
> + /**
> + * Return the cached TransactionManager instance.
> + */
> + public TransactionManager getTransactionManager() throws
> Exception {
> + if (_tm == null) {
> + Context ctx = new InitialContext();
> + try {
> + _tm = (TransactionManager) ctx.lookup(_tmLoc);
> + } finally {
> + ctx.close();
> + }
> }
> - }
> + return _tm;
> + }
> }
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/AbstractBrokerFactory.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
?> view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/AbstractBrokerFactory.java (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/AbstractBrokerFactory.java Sun Feb 11 18:33:05 2007
> @@ -64,7 +64,8 @@
> // configuration
> private final OpenJPAConfiguration _conf;
> private transient boolean _readOnly = false;
> - private transient RuntimeException _closed = null;
> + private transient boolean _closed = false;
> + private transient RuntimeException _closedException = null;
> private Map _userObjects = null;
>
> // internal lock: spec forbids synchronization on this object
> @@ -267,7 +268,7 @@
> * Returns true if this broker factory is closed.
> */
> public boolean isClosed() {
> - return _closed != null;
> + return _closed;
> }
>
> public void close() {
> @@ -297,7 +298,10 @@
> (_conf.getMetaDataRepositoryInstance());
>
> _conf.close();
> - _closed = new IllegalStateException();
> + _closed = true;
> + Log log = _conf.getLog(OpenJPAConfiguration.LOG_RUNTIME);
> + if (log.isTraceEnabled())
> + _closedException = new IllegalStateException();
> } finally {
> unlock();
> }
> @@ -546,12 +550,18 @@
> }
>
> /**
> - * Throw an exception if the factory is closed.
> + * Throw an exception if the factory is closed. The
> exact message and
> + * content of the exception varies whether TRACE is
> enabled or not.
> */
> private void assertOpen() {
> - if (_closed != null)
> - throw new
> InvalidStateException(_loc.get("closed-factory")).
> - setCause(_closed);
> + if (_closed) {
> + if (_closedException == null) // TRACE not enabled
> + throw new InvalidStateException(_loc
> + .get("closed-factory-notrace"));
> + else
> + throw new
> InvalidStateException(_loc.get("closed-factory"))
> + .setCause(_closedException);
> + }
> }
>
> ////////////////////
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?>
view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java Sun Feb 11 18:33:05 2007
> @@ -63,6 +63,7 @@
> import org.apache.openjpa.lib.util.ReferenceHashMap;
> import org.apache.openjpa.lib.util.ReferenceHashSet;
> import org.apache.openjpa.lib.util.ReferenceMap;
> +import
> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> import org.apache.openjpa.lib.util.concurrent.ReentrantLock;
> import org.apache.openjpa.meta.ClassMetaData;
> import org.apache.openjpa.meta.FieldMetaData;
> @@ -138,6 +139,9 @@
>
> private static final Localizer _loc =
> Localizer.forPackage(BrokerImpl.class);
> + // Cache for from/to type assignments
> + private static ConcurrentReferenceHashMap _assignableTypes =
> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> ReferenceMap.WEAK);
>
> // the store manager in use; this may be a
> decorator such as a
> // data cache store manager around the native store manager
> @@ -215,7 +219,8 @@
>
> // status
> private int _flags = 0;
> - private RuntimeException _closed = null;
> + private boolean _closed = false;
> + private RuntimeException _closedException = null;
>
> // event managers
> private TransactionEventManager _transEventManager = null;
> @@ -1096,8 +1101,7 @@
> cls));
> return PCRegistry.newObjectId(cls, (String) val);
> }
> -
> - if
> (meta.getObjectIdType().isAssignableFrom(val.getClass())) {
> + if (isAssignable(meta.getObjectIdType(),
> val.getClass())) {
> if (!meta.isOpenJPAIdentity() &&
> meta.isObjectIdTypeShared())
> return new ObjectId(cls, val);
> return val;
> @@ -1119,6 +1123,37 @@
> }
>
> /**
> + * Cache from/to assignments to avoid
> Class.isAssignableFrom overhead
> + * @param from the target Class
> + * @param to the Class to test
> + * @return true if the "to" class could be assigned to
> "from" class
> + */
> + private boolean isAssignable(Class from, Class to) {
> + boolean isAssignable;
> + ConcurrentReferenceHashMap assignableTo =
> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
> +
> + if (assignableTo != null) { // "to" cache exists...
> + isAssignable = (assignableTo.get(to) != null);
> + if (!isAssignable) { // not in the map yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo.put(to, new Object());
> + }
> + }
> + } else { // no "to" cache yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo = new ConcurrentReferenceHashMap(
> + ReferenceMap.HARD, ReferenceMap.WEAK);
> + _assignableTypes.put(from, assignableTo);
> + assignableTo.put(to, new Object());
> + }
> + }
> + return isAssignable;
> + }
> +
> + /**
> * Create a new state manager for the given oid.
> */
> private StateManagerImpl newStateManagerImpl(Object oid,
> boolean copy) {
> @@ -3969,11 +4004,11 @@
> ///////////
>
> public boolean isClosed() {
> - return _closed != null;
> + return _closed;
> }
>
> public boolean isCloseInvoked() {
> - return _closed != null || (_flags & FLAG_CLOSE_INVOKED) != 0;
> + return _closed || (_flags & FLAG_CLOSE_INVOKED) != 0;
> }
>
> public void close() {
> @@ -4055,8 +4090,10 @@
>
> _lm.close();
> _store.close();
> - _closed = new IllegalStateException();
> _flags = 0;
> + _closed = true;
> + if (_log.isTraceEnabled())
> + _closedException = new IllegalStateException();
>
> if (err != null)
> throw err;
> @@ -4246,11 +4283,19 @@
> /////////
> // Utils
> /////////
> -
> + /**
> + * Throw an exception if the context is closed. The
> exact message and
> + * content of the exception varies whether TRACE is
> enabled or not.
> + */
> public void assertOpen() {
> - if (_closed != null)
> - throw new
> InvalidStateException(_loc.get("closed"), _closed).
> - setFatal(true);
> + if (_closed) {
> + if (_closedException == null) // TRACE not enabled
> + throw new
> InvalidStateException(_loc.get("closed-notrace"))
> + .setFatal(true);
> + else
> + throw new InvalidStateException(_loc.get("closed"),
> + _closedException).setFatal(true);
> + }
> }
>
> public void assertActiveTransaction() {
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/FetchConfigurationImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.jav
a?> view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/FetchConfigurationImpl.java (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/FetchConfigurationImpl.java Sun Feb 11 18:33:05 2007
> @@ -36,6 +36,8 @@
> import org.apache.openjpa.lib.rop.SimpleResultList;
> import org.apache.openjpa.lib.rop.WindowResultList;
> import org.apache.openjpa.lib.util.Localizer;
> +import org.apache.openjpa.lib.util.ReferenceMap;
> +import
> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> import org.apache.openjpa.meta.ClassMetaData;
> import org.apache.openjpa.meta.FetchGroup;
> import org.apache.openjpa.meta.FieldMetaData;
> @@ -58,6 +60,10 @@
> private static final Localizer _loc = Localizer.forPackage
> (FetchConfigurationImpl.class);
>
> + // Cache the from/to isAssignable invocations
> + private static ConcurrentReferenceHashMap _assignableTypes =
> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> ReferenceMap.WEAK);
> +
> /**
> * Configurable state shared throughout a traversal chain.
> */
> @@ -613,11 +619,37 @@
> }
>
> /**
> - * Whether either of the two types is assignable from the other.
> + * Whether either of the two types is assignable from
> the other. Optimize
> + * for the repeat calls with similar parameters by
> caching the from/to
> + * type parameters.
> */
> - private static boolean isAssignable(Class c1, Class c2) {
> - return c1 != null && c2 != null
> - && (c1.isAssignableFrom(c2) || c2.isAssignableFrom(c1));
> + private static boolean isAssignable(Class from, Class to) {
> + boolean isAssignable;
> +
> + if (from == null || to == null)
> + return false;
> + ConcurrentReferenceHashMap assignableTo =
> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
> +
> + if (assignableTo != null) { // "to" cache exists...
> + isAssignable = (assignableTo.get(to) != null);
> + if (!isAssignable) { // not in the map yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo.put(to, new Object());
> + }
> + }
> + } else { // no "to" cache yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo = new ConcurrentReferenceHashMap(
> + ReferenceMap.HARD, ReferenceMap.WEAK);
> + _assignableTypes.put(from, assignableTo);
> + assignableTo.put(to, new Object());
> + }
> + }
> +
> + return isAssignable;
> }
>
> /**
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/util/OpenJPAId.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/util/OpenJPAId.java?view=diff&rev
=> 506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/util/OpenJPAId.java (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/util/OpenJPAId.java Sun Feb 11 18:33:05 2007
> @@ -17,6 +17,10 @@
>
> import java.io.Serializable;
>
> +import org.apache.openjpa.lib.util.ReferenceHashSet;
> +import org.apache.openjpa.lib.util.ReferenceMap;
> +import
> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
> +
> /**
> * Identity class extended by builtin OpenJPA identity objects.
> *
> @@ -31,6 +35,9 @@
> // type has his based on the least-derived non-object
> class so that
> // user-given ids with non-exact types match ids with exact types
> private transient int _typeHash = 0;
> + // cache the types' generated hashcodes
> + private static ConcurrentReferenceHashMap _typeCache =
> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> ReferenceMap.WEAK);
>
> protected OpenJPAId() {
> }
> @@ -82,13 +89,25 @@
> */
> protected abstract boolean idEquals(OpenJPAId other);
>
> + /**
> + * Generate the hashcode for this Id. Cache the type's
> generated hashcode
> + * so that it doesn't have to be generated each time.
> + */
> public int hashCode() {
> if (_typeHash == 0) {
> - Class base = type;
> - while (base.getSuperclass() != null
> - && base.getSuperclass() != Object.class)
> - base = base.getSuperclass();
> - _typeHash = base.hashCode();
> + Integer typeHashInt = (Integer) _typeCache.get(type);
> + if (typeHashInt == null) {
> + Class base = type;
> + Class superclass = base.getSuperclass();
> + while (superclass != null && superclass !=
> Object.class) {
> + base = base.getSuperclass();
> + superclass = base.getSuperclass();
> + }
> + _typeHash = base.hashCode();
> + _typeCache.put(type, new Integer(_typeHash));
> + } else {
> + _typeHash = typeHashInt.intValue();
> + }
> }
> return _typeHash ^ idHash();
> }
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
apache/openjpa/kernel/localizer.properties
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties?
> view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
apache/openjpa/kernel/localizer.properties (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/
apache/openjpa/kernel/localizer.properties Sun Feb 11 18:33:05 2007
> @@ -94,8 +94,13 @@
> active: This operation cannot be performed while a
> Transaction is active.
> closed: The context has been closed. The stack trace at which the \
> context was closed is held in the embedded exception.
> +closed-notrace: The context has been closed. The stack
> trace at which the \
> + context was closed is available if Runtime=TRACE
> logging is enabled.
> closed-factory: The factory has been closed. The stack trace at \
> which the factory was closed is held in the embedded exception.
> +closed-factory-notrace: The factory has been closed. The
> stack trace at \
> + which the factory was closed is available if
> Runtime=TRACE logging is \
> + enabled.
> non-trans-read: To perform reads on persistent data outside
> of a transaction, \
> the "NontransactionalRead" property must be set on the
> Transaction.
> non-trans-write: To perform writes on persistent data outside of a \
>
> Modified:
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
penjpa/lib/conf/ObjectValue.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-l
ib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?>
view=diff&rev=506230&r1=506229&r2=506230
> ==============================================================
> ================
> ---
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
penjpa/lib/conf/ObjectValue.java (original)
> +++
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/o
penjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
> @@ -17,6 +17,8 @@
>
> import org.apache.commons.lang.ObjectUtils;
> import org.apache.openjpa.lib.util.Localizer;
> +import org.apache.openjpa.lib.util.ReferenceMap;
> +import
> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>
> /**
> * An object {@link Value}.
> @@ -28,6 +30,10 @@
> private static final Localizer _loc = Localizer.forPackage
> (ObjectValue.class);
>
> + // cache the types' classloader
> + private static ConcurrentReferenceHashMap _classloaderCache =
> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> ReferenceMap.WEAK);
> +
> private Object _value = null;
>
> public ObjectValue(String prop) {
> @@ -81,10 +87,16 @@
> * Allow subclasses to instantiate additional plugins.
> This method does
> * not perform configuration.
> */
> - public Object newInstance(String clsName, Class type,
> - Configuration conf, boolean fatal) {
> - return Configurations.newInstance(clsName, this, conf,
> - type.getClassLoader(), fatal);
> + public Object newInstance(String clsName, Class type,
> Configuration conf,
> + boolean fatal) {
> + ClassLoader cl = (ClassLoader) _classloaderCache.get(type);
> + if (cl == null) {
> + cl = type.getClassLoader();
> + if (cl != null) { // System classloader is
> returned as null
> + _classloaderCache.put(type, cl);
> + }
> + }
> + return Configurations.newInstance(clsName, this,
> conf, cl, fatal);
> }
>
> public Class getValueType() {
>
>
>
Re: svn commit: r506230 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/ee/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/util/ openjpa-kernel/src/main/resources/org/a...
Posted by Abe White <aw...@bea.com>.
> ======================================================================
> ========
> --- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/
> openjpa/ee/JNDIManagedRuntime.java (original)
> +++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/
> openjpa/ee/JNDIManagedRuntime.java Sun Feb 11 18:33:05 2007
> @@ -29,6 +29,7 @@
> implements ManagedRuntime {
>
> private String _tmLoc = "java:/TransactionManager";
> + private static TransactionManager _tm;
Whoa, I didn't think you meant caching the TM statically. That has
to be backed out. You can cache it in an instance variable, but not
statically. Nothing should prevent someone having two different
BrokerFactories accessing two different TMs at two different JNDI
locations.
BrokerImpl:
> + * Cache from/to assignments to avoid Class.isAssignableFrom
> overhead
> + * @param from the target Class
> + * @param to the Class to test
> + * @return true if the "to" class could be assigned to "from"
> class
> + */
> + private boolean isAssignable(Class from, Class to) {
> + boolean isAssignable;
> + ConcurrentReferenceHashMap assignableTo =
> + (ConcurrentReferenceHashMap) _assignableTypes.get(from);
> +
> + if (assignableTo != null) { // "to" cache exists...
> + isAssignable = (assignableTo.get(to) != null);
> + if (!isAssignable) { // not in the map yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo.put(to, new Object());
> + }
> + }
> + } else { // no "to" cache yet...
> + isAssignable = from.isAssignableFrom(to);
> + if (isAssignable) {
> + assignableTo = new ConcurrentReferenceHashMap(
> + ReferenceMap.HARD, ReferenceMap.WEAK);
> + _assignableTypes.put(from, assignableTo);
> + assignableTo.put(to, new Object());
> + }
> + }
> + return isAssignable;
> + }
This code could be simplified a lot. Also, I don't understand what
you're trying to do from a memory management perspective. For the
_assignableTypes member you've got the Class keys using hard refs and
the Map values using weak refs. No outside code references the Map
values, so all entries should be eligible for GC pretty much
immediately. The way reference hash maps work prevents them from
expunging stale entries except on mutators, but still... every time a
new entry is added, all the old entries should be getting GC'd and
removed. Same for the individual Map values, which again map a hard
class ref to an unreferenced object value with a weak ref. Basically
the whole map-of-maps system should never contain more than one entry
total after a GC run and a mutation.
I'd really like to see you run your tests under a different JVM,
because it seems to me like (a) this shouldn't be necessary in the
first place, and (b) if this is working, it's again only because of
some JVM particulars or GC timing particulars or testing particulars
(I've seen profilers skew results in random ways like this) or even a
bug in ConcurrentReferenceHashMap.
The same goes for all the repeat logic in FetchConfigurationImpl.
And if we keep this code or some variant of it, I strongly suggest
moving it to a common place like ImplHelper.
> + /**
> + * Generate the hashcode for this Id. Cache the type's
> generated hashcode
> + * so that it doesn't have to be generated each time.
> + */
> public int hashCode() {
> if (_typeHash == 0) {
> - Class base = type;
> - while (base.getSuperclass() != null
> - && base.getSuperclass() != Object.class)
> - base = base.getSuperclass();
> - _typeHash = base.hashCode();
> + Integer typeHashInt = (Integer) _typeCache.get(type);
> + if (typeHashInt == null) {
> + Class base = type;
> + Class superclass = base.getSuperclass();
> + while (superclass != null && superclass !=
> Object.class) {
> + base = base.getSuperclass();
> + superclass = base.getSuperclass();
> + }
> + _typeHash = base.hashCode();
> + _typeCache.put(type, new Integer(_typeHash));
> + } else {
> + _typeHash = typeHashInt.intValue();
> + }
> }
> return _typeHash ^ idHash();
> }
Once again, you're mapping a hard Class ref to a value with no
outside references held in a weak ref. Once again that means the
entry should be immediately eligible for GC, and therefore should be
removed on the next mutation of the cache, subject to GC timing. And
again I'd like to know what your JVM is doing to make Class.hashCode
take an appreciable amount of time. Aren't Class instances supposed
to be singletons? What if we just used System.identityHashCode(cls)?
> Modified: incubator/openjpa/trunk/openjpa-lib/src/main/java/org/
> apache/openjpa/lib/conf/ObjectValue.java
> URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-
> lib/src/main/java/org/apache/openjpa/lib/conf/ObjectValue.java?
> view=diff&rev=506230&r1=506229&r2=506230
> ======================================================================
> ========
> --- incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/
> openjpa/lib/conf/ObjectValue.java (original)
> +++ incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/
> openjpa/lib/conf/ObjectValue.java Sun Feb 11 18:33:05 2007
> @@ -17,6 +17,8 @@
>
> import org.apache.commons.lang.ObjectUtils;
> import org.apache.openjpa.lib.util.Localizer;
> +import org.apache.openjpa.lib.util.ReferenceMap;
> +import
> org.apache.openjpa.lib.util.concurrent.ConcurrentReferenceHashMap;
>
> /**
> * An object {@link Value}.
> @@ -28,6 +30,10 @@
> private static final Localizer _loc = Localizer.forPackage
> (ObjectValue.class);
>
> + // cache the types' classloader
> + private static ConcurrentReferenceHashMap _classloaderCache =
> + new ConcurrentReferenceHashMap(ReferenceMap.HARD,
> ReferenceMap.WEAK);
This maps a hard Class ref to a weak ClassLoader ref. Given that a
Class references its ClassLoader (or is supposed to -- again I wonder
what the hell the JVM you're using is doing where
Class.getClassLoader is taking a long time), no entries will ever
expire from this map.
Have you tried running your benchmarks without all the caching of
assignables and classloaders and hashcodes (all Class methods, btw)
and just the other improvements? Or with any other JVM?
_______________________________________________________________________
Notice: This email message, together with any attachments, may contain
information of BEA Systems, Inc., its subsidiaries and affiliated
entities, that may be confidential, proprietary, copyrighted and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.