diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java index 560c15dfd..fa6891f50 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java @@ -162,12 +162,15 @@ public class DataSourceFactory { decorators.addAll(decs); } - // logging decorator - LoggingConnectionDecorator lcd = new LoggingConnectionDecorator(); - Configurations.configureInstance(lcd, conf, opts); - lcd.getLogs().setJDBCLog(jdbcLog); - lcd.getLogs().setSQLLog(sqlLog); - decorators.add(lcd); + if (jdbcLog.isTraceEnabled() || sqlLog.isTraceEnabled()) { + // logging decorator + LoggingConnectionDecorator lcd = + new LoggingConnectionDecorator(); + Configurations.configureInstance(lcd, conf, opts); + lcd.getLogs().setJDBCLog(jdbcLog); + lcd.getLogs().setSQLLog(sqlLog); + decorators.add(lcd); + } dds.addDecorators(decorators); return dds; diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java index 40c00ca4f..9fc3e0262 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java @@ -349,14 +349,15 @@ public interface FetchConfiguration /** * Affirms if the given fields require to be fetched in the context of * the given fetch group set. Returns a BitSet that contains one of - * {@link #FETCH_NONE}, {@link #FETCH_LOAD}, {@link FETCH_REF} for each + * {@link #FETCH_NONE}, {@link #FETCH_LOAD}, {@link #FETCH_REF} for each * field. * * @param fgs fetch group set * @param fmds array of fields to be examined - * @return BitSet that indicates whether fetches are required or not + * @return BitSet that indicates whether fetches are required or not, or + * null if no fields require a fetch. */ - public BitSet requiresFetch(Set fgs, FieldMetaData[] fmds ); + public BitSet requiresFetch(Set fgs, FieldMetaData[] fmds); /** * Return false if we know that the object being fetched with this diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java index 17e0cc5d2..fe2cfd8f2 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java @@ -537,7 +537,7 @@ public class FetchConfigurationImpl } public BitSet requiresFetch(Set fgs, FieldMetaData[] fmds) { - BitSet fields = new BitSet(fgs.size()); + BitSet fields = null; Iterator itr = fgs.iterator(); while (itr.hasNext()) { fields = includes((FieldMetaData) itr.next(), fmds, fields); @@ -594,8 +594,10 @@ public class FetchConfigurationImpl if ((fmd.isInDefaultFetchGroup() && hasFetchGroup(FetchGroup.NAME_DEFAULT)) || hasFetchGroup(FetchGroup.NAME_ALL) || hasField(fmd.getFullName(false))) { - if (indirectFetch(fmd) != FETCH_NONE) + if (indirectFetch(fmd) != FETCH_NONE) { + fields = new BitSet(fmds.length); fields.set(fmd.getIndex()); + } return fields; } // now we need to see if this field associates with @@ -603,8 +605,11 @@ public class FetchConfigurationImpl String[] fgs = fmd.getCustomFetchGroups(); for (int i = 0; i < fgs.length; i++) { if (hasFetchGroup(fgs[i])) { - if (indirectFetch(fmd) != FETCH_NONE) + if (indirectFetch(fmd) != FETCH_NONE) { + if (fields == null) + fields = new BitSet(fmds.length); fields.set(fmd.getIndex()); + } // check whether this field has a loadFetchGroup // if it has a LoadFetchGroup, then we need to get // all the fields that associate with this LoadFetchGroup @@ -614,8 +619,11 @@ public class FetchConfigurationImpl // merge the loadFetchGroup fields to the retuned fields. if (fldIndex != null && !fldIndex.isEmpty()) { for (int j = 0; j < fldIndex.length(); j++) - if (fldIndex.get(j)) + if (fldIndex.get(j)) { + if (fields == null) + fields = new BitSet(fmds.length); fields.set(j); + } } } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java index 0e470db45..3766a18b5 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java @@ -83,7 +83,7 @@ public class QueryImpl private transient ClassLoader _loader = null; // query has its own internal lock - private final ReentrantLock _lock = new ReentrantLock(); + private final ReentrantLock _lock; // unparsed state private Class _class = null; @@ -134,6 +134,11 @@ public class QueryImpl _fc = (FetchConfiguration) broker.getFetchConfiguration().clone(); _log = broker.getConfiguration().getLog(OpenJPAConfiguration.LOG_QUERY); _storeQuery.setContext(this); + + if (_broker != null && _broker.getMultithreaded()) + _lock = new ReentrantLock(); + else + _lock = null; } /** @@ -934,12 +939,6 @@ public class QueryImpl * Return whether we should execute this query in memory. */ private boolean isInMemory(int operation) { - // if no candidates, create candidate extent - if (_collection == null && _extent == null && _class != null) { - _extent = _broker.newExtent(_class, _subclasses); - _extent.setIgnoreChanges(_ignoreChanges); - } - // if there are any dirty instances in the current trans that are // involved in this query, we have to execute in memory or flush boolean inMem = !_storeQuery.supportsDataStoreExecution() @@ -1538,12 +1537,12 @@ public class QueryImpl } public void lock() { - if (_broker == null || _broker.getMultithreaded()) + if (_lock != null) _lock.lock(); } public void unlock() { - if (_lock.isLocked()) + if (_lock != null && _lock.isLocked()) _lock.unlock(); } diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/DataSourceLogs.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/DataSourceLogs.java index b6ad97da6..ca3f6efcc 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/DataSourceLogs.java +++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/DataSourceLogs.java @@ -41,11 +41,6 @@ public class DataSourceLogs { public DataSourceLogs() { } - public DataSourceLogs(Log jdbcLog, Log sqlLog) { - _jdbcLog = jdbcLog; - _sqlLog = sqlLog; - } - /** * The log to write JDBC messages to. */ diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestSelectReuse.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestSelectReuse.java new file mode 100644 index 000000000..f4951efcb --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestSelectReuse.java @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.jdbc.kernel; + +import java.util.Map; +import javax.persistence.EntityManager; + +import org.apache.openjpa.jdbc.conf.JDBCConfiguration; +import org.apache.openjpa.kernel.Broker; +import org.apache.openjpa.kernel.FetchConfiguration; +import org.apache.openjpa.persistence.JPAFacadeHelper; +import org.apache.openjpa.persistence.query.SimpleEntity; +import org.apache.openjpa.persistence.test.SingleEMTestCase; + +public class TestSelectReuse + extends SingleEMTestCase { + + private long id; + + public void setUp() { + setUp(SimpleEntity.class, CLEAR_TABLES, + "openjpa.ConnectionRetainMode", "always"); + + SimpleEntity se = new SimpleEntity(); + se.setName("foo"); + se.setValue("bar"); + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist(se); + em.getTransaction().commit(); + id = se.getId(); + em.close(); + } + + public void testFetchConfigurationEqualsHashCode() { + Broker b = JPAFacadeHelper.toBroker(em); + FetchConfiguration fc = b.getFetchConfiguration(); + FetchConfiguration clone = (FetchConfiguration) fc.clone(); + clone.setContext(null); + assertEquals(fc.hashCode(), clone.hashCode()); + assertEquals(fc, clone); + } + + public void testSelectReuseWithinSingleEM() { + Broker b = JPAFacadeHelper.toBroker(em); +// Map selects = ((JDBCConfiguration) b.getConfiguration()) +// .getSelectCacheInstance(); +// selects.clear(); + SimpleEntity se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + em.clear(); + se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + em.clear(); + se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + } + + public void testSelectReuseAcrossMultipleEMs() { + Broker b = JPAFacadeHelper.toBroker(em); +// Map selects = ((JDBCConfiguration) b.getConfiguration()) +// .getSelectCacheInstance(); +// selects.clear(); + SimpleEntity se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + em.close(); + em = emf.createEntityManager(); + se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + em.close(); + em = emf.createEntityManager(); + se = em.find(SimpleEntity.class, id); +// assertEquals(1, selects.size()); + } + + public void testPerformanceBenefit() { + int count = 100000; + perfTest(count); + queryTest(count); + long start = System.currentTimeMillis(); + perfTest(count); + System.out.println("time for " + count + " runs: " + + (System.currentTimeMillis() - start)); + start = System.currentTimeMillis(); + queryTest(count); + System.out.println("time for " + count + " runs: " + + (System.currentTimeMillis() - start)); + } + + public static void main(String... args) throws Exception { + TestSelectReuse tsr = new TestSelectReuse(); + tsr.setUp(); + tsr.testPerformanceBenefit(); + tsr.tearDown(); + } + + private void perfTest(int count) { + for (int i = 0; i < count; i++) { + SimpleEntity se = em.find(SimpleEntity.class, id); + assertNotNull(se); + em.clear(); +// em.close(); +// em = emf.createEntityManager(); + } + } + + private void queryTest(int count) { + for (int i = 0; i < count; i++) { + SimpleEntity se = (SimpleEntity) em + .createQuery("select o from simple o where o.id = :id") + .setParameter("id", id) + .getSingleResult(); + assertNotNull(se); + em.clear(); + } + } +} diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java index de8ea6de9..328c7dd9c 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java @@ -70,6 +70,9 @@ public class EntityManagerImpl private FetchPlan _fetch = null; private static final Object[] EMPTY_OBJECTS = new Object[0]; + private RuntimeExceptionTranslator ret = + PersistenceExceptions.getRollbackTranslator(this); + /** * Constructor; supply factory and delegate. */ @@ -813,7 +816,7 @@ public class EntityManagerImpl public OpenJPAQuery createQuery(String language, String query) { assertNotCloseInvoked(); - return new QueryImpl(this, _broker.newQuery(language, query)); + return new QueryImpl(this, ret, _broker.newQuery(language, query)); } public OpenJPAQuery createQuery(Query query) { @@ -821,7 +824,7 @@ public class EntityManagerImpl return createQuery((String) null); assertNotCloseInvoked(); org.apache.openjpa.kernel.Query q = ((QueryImpl) query).getDelegate(); - return new QueryImpl(this, _broker.newQuery(q.getLanguage(), + return new QueryImpl(this, ret, _broker.newQuery(q.getLanguage(), q)); } @@ -837,7 +840,7 @@ public class EntityManagerImpl meta.setInto(del); del.compile(); - OpenJPAQuery q = new QueryImpl(this, del); + OpenJPAQuery q = new QueryImpl(this, ret, del); String[] hints = meta.getHintKeys(); Object[] values = meta.getHintValues(); for (int i = 0; i < hints.length; i++) @@ -863,7 +866,7 @@ public class EntityManagerImpl org.apache.openjpa.kernel.Query kernelQuery = _broker.newQuery( QueryLanguages.LANG_SQL, query); kernelQuery.setResultMapping(null, mappingName); - return new QueryImpl(this, kernelQuery); + return new QueryImpl(this, ret, kernelQuery); } /** diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java index 8e59a84ae..e91b5ca52 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java @@ -45,6 +45,7 @@ import org.apache.openjpa.kernel.exps.FilterListener; import org.apache.openjpa.lib.rop.ResultList; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.util.ImplHelper; +import org.apache.openjpa.util.RuntimeExceptionTranslator; /** * Implementation of {@link Query} interface. @@ -71,11 +72,10 @@ public class QueryImpl /** * Constructor; supply factory and delegate. */ - public QueryImpl(EntityManagerImpl em, + public QueryImpl(EntityManagerImpl em, RuntimeExceptionTranslator ret, org.apache.openjpa.kernel.Query query) { _em = em; - _query = new DelegatingQuery(query, - PersistenceExceptions.getRollbackTranslator(em)); + _query = new DelegatingQuery(query, ret); } /**