Improvements to ExecutionContext.findObject and Level 1 cache handling.

src/main/java/org/datanucleus/ExecutionContextImpl.java

@@ -42,6 +42,8 @@
 import org.datanucleus.cache.Level2Cache;
 import org.datanucleus.cache.SoftRefCache;
 import org.datanucleus.cache.StrongRefCache;
+import org.datanucleus.cache.SupportsConcurrentModificationsIteration;
+import org.datanucleus.cache.TieredLevel1Cache;
 import org.datanucleus.cache.WeakRefCache;
 import org.datanucleus.enhancement.Persistable;
 import org.datanucleus.enhancer.ImplementationCreator;
@@ -84,8 +86,11 @@
 import org.datanucleus.state.LockMode;
 import org.datanucleus.state.DNStateManager;
 import org.datanucleus.state.RelationshipManager;
+import org.datanucleus.state.StateManagerImpl;
 import org.datanucleus.store.FieldValues;
+import org.datanucleus.store.StorePersistenceHandler;
 import org.datanucleus.store.StorePersistenceHandler.PersistenceBatchType;
+import org.datanucleus.store.ValidatingStorePersistenceHandler;
 import org.datanucleus.store.query.Extent;
 import org.datanucleus.store.types.converters.TypeConversionHelper;
 import org.datanucleus.store.types.scostore.Store;
@@ -512,8 +517,13 @@ else if (closeActionTxAction.equals("rollback"))
         if (cache != null && !cache.isEmpty())
         {
             // Clear out the cache (use separate list since sm.disconnect will remove the object from "cache" so we avoid any ConcurrentModification issues)
-            Collection<DNStateManager> cachedSMsClone = new HashSet<>(cache.values());
-            for (DNStateManager sm : cachedSMsClone)
+            Collection<DNStateManager> cachedSMs = cache.values();
+            if (!(cachedSMs instanceof SupportsConcurrentModificationsIteration))
+            {
+                cachedSMs = new HashSet<>(cachedSMs);
+            }
+
+            for (DNStateManager sm : cachedSMs)
             {
                 if (sm != null)
                 {
@@ -3066,10 +3076,17 @@ public Persistable findObject(Object id, FieldValues fv, Class cls, boolean igno
             // Check if an object exists in the L1/L2 caches for this id
             pc = getObjectFromCache(id);
         }
+        else
+        {
+            // ALWAYS look in level 1 as otherwise we might end up with two PC
+            // instances for same object
+            pc = getObjectFromLevel1Cache(id);
+        }
 
-        if (pc == null)
+        if (pc == null && fv == null)
         {
             // Find direct from the datastore if supported
+            // Only invoke findObject from datastore if we DON'T already have a way to provide field values, e.g. from result set.
             pc = (Persistable) getStoreManager().getPersistenceHandler().findObject(this, id);
         }
 
@@ -3370,6 +3387,8 @@ public Persistable findObject(Object id, boolean validate, boolean checkInherita
             throw new NucleusUserException(Localiser.msg("010044"));
         }
 
+        boolean foundViaPersistenceHandler = false;
+
         IdentityStringTranslator translator = getNucleusContext().getIdentityManager().getIdentityStringTranslator();
         if (translator != null && id instanceof String)
         {
@@ -3407,6 +3426,7 @@ public Persistable findObject(Object id, boolean validate, boolean checkInherita
             pc = (Persistable) getStoreManager().getPersistenceHandler().findObject(this, id);
             if (pc != null)
             {
+                foundViaPersistenceHandler = true;
                 sm = findStateManager(pc);
                 putObjectIntoLevel1Cache(sm);
                 putObjectIntoLevel2Cache(sm, false);
@@ -3469,7 +3489,20 @@ public Persistable findObject(Object id, boolean validate, boolean checkInherita
 
             try
             {
-                sm.validate();
+                final StorePersistenceHandler persistenceHandler = getStoreManager().getPersistenceHandler();
+                if (persistenceHandler instanceof ValidatingStorePersistenceHandler)
+                {
+                    // If the object was loaded in findObject from persistence handler in store manager
+                    // then it might not be necessary to check for its existence again in DB
+                    // by calling sm.validate().
+                    // We leave this decision to the persistence handler if it supports this optimization.
+                    ((ValidatingStorePersistenceHandler) persistenceHandler).validate(sm, foundViaPersistenceHandler);
+                }
+                else
+                {
+                    sm.validate();
+                }
+
 
                 if (sm.getObject() != pc)
                 {
@@ -4061,6 +4094,15 @@ public void postBegin()
         }
     }
 
+    private static Iterable<? extends DNStateManager> getMostlyNonHollowValues(Level1Cache cache) {
+        if (cache instanceof TieredLevel1Cache) {
+            TieredLevel1Cache tieredCache = (TieredLevel1Cache) cache;
+            return tieredCache.hotValues();
+        } else {
+            return new HashSet<>(cache.values());
+        }
+    }
+
     /**
      * Method to perform any pre-commit checks.
      */
@@ -4069,11 +4111,11 @@ public void preCommit()
         if (cache != null && !cache.isEmpty())
         {
             // Check for objects that are managed but not dirty, yet require a version update
-            Collection<DNStateManager> cachedSMs = new HashSet<>(cache.values());
+            final Iterable<? extends DNStateManager> cachedSMs = getMostlyNonHollowValues(cache);
             for (DNStateManager cachedSM : cachedSMs)
             {
                 LockMode lockMode = getLockManager().getLockMode(cachedSM);
-                if (cachedSM != null && cachedSM.isFlushedToDatastore() && cachedSM.getClassMetaData().isVersioned() && 
+                if (cachedSM != null && cachedSM.isFlushedToDatastore() && isVersioned(cachedSM) &&
                         (lockMode == LockMode.LOCK_OPTIMISTIC_WRITE || lockMode == LockMode.LOCK_PESSIMISTIC_WRITE))
                 {
                     // Not dirty, but locking requires a version update, so force it
@@ -4129,6 +4171,15 @@ public void preCommit()
         }
     }
 
+    private static boolean isVersioned(DNStateManager cachedSM)
+    {
+        if (cachedSM instanceof StateManagerImpl)
+        {
+            return ((StateManagerImpl) cachedSM).isVersioned();
+        }
+        return cachedSM.getClassMetaData().isVersioned();
+    }
+
     /**
      * Accessor for whether the object with this identity is modified in the current transaction.
      * Only returns true when using the L2 cache and the object has been modified during the txn.

src/main/java/org/datanucleus/cache/SupportsConcurrentModificationsIteration.java

@@ -0,0 +1,24 @@
+/**********************************************************************
+ Copyright (c) 2006 Andy Jefferson and others. All rights reserved.
+ Licensed 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.datanucleus.cache;
+
+/**
+ * Marker interface for collections where it's possible to make modifications while iterating.
+ * When used for Level 1 cache implementations there is a considerable performance benefit
+ * when closing an execution context with many objects in Level 1 cache.
+ */
+public interface SupportsConcurrentModificationsIteration
+{
+}

src/main/java/org/datanucleus/cache/TieredLevel1Cache.java

@@ -0,0 +1,31 @@
+/**********************************************************************
+ Copyright (c) 2006 Andy Jefferson and others. All rights reserved.
+ Licensed 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.datanucleus.cache;
+
+import org.datanucleus.state.DNStateManager;
+
+/**
+ * A marker interface for Level1Cache for optimizing commit.
+ * During commit we are not interested in hollow PC objects in Level1
+ * cache - Level1 caches implementing this interface could optimize
+ * the filtering of finding non-hollow PC objects.
+ */
+public interface TieredLevel1Cache extends Level1Cache {
+    /**
+     * Return non-hollow state managers that could be considered as dirty during commit phase.
+     * @return non-hollow state managers to consider during commit phase
+     */
+    Iterable<? extends DNStateManager> hotValues();
+}

src/main/java/org/datanucleus/state/StateManagerImpl.java

@@ -1599,7 +1599,7 @@ protected void loadFieldsFromDatastore(int[] fieldNumbers)
 
         // Add on version field if not currently set and using version field (surrogate will be added automatically by the query if required)
         int[] fieldNumbersToFetch = fieldNumbers;
-        if (cmd.isVersioned())
+        if (isVersioned())
         {
             VersionMetaData vermd = cmd.getVersionMetaDataForClass();
             if (vermd != null && vermd.getMemberName() != null)
@@ -2355,7 +2355,7 @@ public Object getVersion(Persistable pc)
     {
         if (pc == myPC)
         {
-            if (transactionalVersion == null && cmd.isVersioned())
+            if (transactionalVersion == null && isVersioned())
             {
                 // If the object is versioned and no version is loaded (e.g obtained via findObject without loading fields) and in a state where we need it then pull in the version
                 VersionMetaData vermd = cmd.getVersionMetaDataForClass();
@@ -2387,7 +2387,7 @@ public Object getVersion(Persistable pc)
      */
     public boolean isVersionLoaded()
     {
-        if (cmd.isVersioned())
+        if (isVersioned())
         {
             return transactionalVersion != null;
         }
@@ -2614,7 +2614,7 @@ public void setBooleanField(Persistable pc, int fieldNumber, boolean currentValu
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2661,7 +2661,7 @@ public void setByteField(Persistable pc, int fieldNumber, byte currentValue, byt
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2708,7 +2708,7 @@ public void setCharField(Persistable pc, int fieldNumber, char currentValue, cha
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2755,7 +2755,7 @@ public void setDoubleField(Persistable pc, int fieldNumber, double currentValue,
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2802,7 +2802,7 @@ public void setFloatField(Persistable pc, int fieldNumber, float currentValue, f
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2849,7 +2849,7 @@ public void setIntField(Persistable pc, int fieldNumber, int currentValue, int n
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2896,7 +2896,7 @@ public void setLongField(Persistable pc, int fieldNumber, long currentValue, lon
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2943,7 +2943,7 @@ public void setShortField(Persistable pc, int fieldNumber, short currentValue, s
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -2990,7 +2990,7 @@ public void setStringField(Persistable pc, int fieldNumber, String currentValue,
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -3048,7 +3048,7 @@ public void setObjectField(Persistable pc, int fieldNumber, Object currentValue,
         }
         else if (myLC != null)
         {
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 // Not got version but should have
                 loadUnloadedFieldsInFetchPlanAndVersion();
@@ -3970,7 +3970,7 @@ public void loadUnloadedFieldsInFetchPlan()
      */
     protected void loadUnloadedFieldsInFetchPlanAndVersion()
     {
-        if (!cmd.isVersioned())
+        if (!isVersioned())
         {
             loadUnloadedFieldsInFetchPlan();
         }
@@ -5573,7 +5573,7 @@ public void validate()
             }
 
             boolean versionNeedsLoading = false;
-            if (cmd.isVersioned() && transactionalVersion == null)
+            if (isVersioned() && transactionalVersion == null)
             {
                 versionNeedsLoading = true;
             }
@@ -6240,4 +6240,9 @@ else if (!cmd.getFullClassName().equals(className))
             myID = myEC.getNucleusContext().getIdentityManager().getApplicationId(myPC, cmd);
         }
     }
+
+    public boolean isVersioned()
+    {
+        return cmd.isVersioned();
+    }
 }

src/main/java/org/datanucleus/store/ValidatingStorePersistenceHandler.java

@@ -0,0 +1,39 @@
+/**********************************************************************
+ Copyright (c) 2009 Erik Bengtson and others. All rights reserved.
+ Licensed 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.datanucleus.store;
+
+import org.datanucleus.state.DNStateManager;
+
+/**
+ * Marker interface for StorePersistenceHandler implementations.
+ * Implementing this interface enabled the persistence handler
+ * to decide what to do if EC.findObject is requested to validate
+ * the existence of a found object.
+ * If the object was loaded in findObject from the persistence handler
+ * then it might not be necessary to check for its existence again in DB
+ * by calling sm.validate().
+ * We leave this decision to the persistence handler if it supports this optimization.
+ */
+public interface ValidatingStorePersistenceHandler
+{
+    /**
+     * Validates whether the supplied state manager instance exists in the datastore.
+     * If the instance does not exist in the datastore, this method will fail raising a NucleusObjectNotFoundException.
+     *
+     * @param sm the state manager instance to check
+     * @param readFromPersistenceHandler whether object was read from persistence handler in datastore.
+     */
+    void validate(DNStateManager sm, boolean readFromPersistenceHandler);
+}