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: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
>> 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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() {
>>
>>
>>
