Merge pull request #607 from dimagi/ctsims/fixture_performance_improv…

…ement Performance: Allow bulk lookups with indexed fixtures
dimagi · Jul 17, 2017 · d66a0a4 · d66a0a4
2 parents 6a5d430 + eb7afb0
commit d66a0a4
Show file tree

Hide file tree

Showing 20 changed files with 784 additions and 100 deletions.
diff --git a/src/main/java/org/commcare/cases/instance/CaseInstanceTreeElement.java b/src/main/java/org/commcare/cases/instance/CaseInstanceTreeElement.java
@@ -71,4 +71,8 @@ protected Hashtable<XPathPathExpr, String> getStorageIndexMap() {
 
         return indices;
     }
+
+    protected String getStorageCacheName() {
+        return CaseInstanceTreeElement.MODEL_NAME;
+    }
 }
diff --git a/src/main/java/org/commcare/cases/instance/IndexedFixtureChildElement.java b/src/main/java/org/commcare/cases/instance/IndexedFixtureChildElement.java
@@ -2,6 +2,7 @@
 
 import org.commcare.cases.model.StorageIndexedTreeElementModel;
 import org.commcare.cases.query.QueryContext;
+import org.commcare.cases.query.QuerySensitive;
 import org.javarosa.core.model.instance.TreeElement;
 import org.javarosa.core.model.instance.TreeReference;
 
@@ -12,7 +13,7 @@
  *
  * @author Phillip Mates ([email protected])
  */
-public class IndexedFixtureChildElement extends StorageBackedChildElement<StorageIndexedTreeElementModel> {
+public class IndexedFixtureChildElement extends StorageBackedChildElement<StorageIndexedTreeElementModel> implements QuerySensitive{
     private TreeElement empty;
 
     protected IndexedFixtureChildElement(StorageInstanceTreeElement<StorageIndexedTreeElementModel, ?> parent,
@@ -65,4 +66,9 @@ public static IndexedFixtureChildElement buildFixtureChildTemplate(IndexedFixtur
         template.empty.setMult(TreeReference.INDEX_TEMPLATE);
         return template;
     }
+
+    @Override
+    public void prepareForUseInCurrentContext(QueryContext queryContext) {
+        cache(queryContext);
+    }
 }
diff --git a/src/main/java/org/commcare/cases/instance/IndexedFixtureInstanceTreeElement.java b/src/main/java/org/commcare/cases/instance/IndexedFixtureInstanceTreeElement.java
@@ -21,11 +21,13 @@
 public class IndexedFixtureInstanceTreeElement
         extends StorageInstanceTreeElement<StorageIndexedTreeElementModel, IndexedFixtureChildElement> {
     private Hashtable<XPathPathExpr, String> storageIndexMap = null;
+    private String cacheKey;
 
     private IndexedFixtureInstanceTreeElement(AbstractTreeElement instanceRoot,
                                               IStorageUtilityIndexed<StorageIndexedTreeElementModel> storage,
                                               String modelName, String childName) {
         super(instanceRoot, storage, modelName, childName);
+        cacheKey = modelName + "|" + childName;
     }
 
     public static IndexedFixtureInstanceTreeElement get(UserSandbox sandbox,
@@ -68,4 +70,8 @@ protected Hashtable<XPathPathExpr, String> getStorageIndexMap() {
 
         return storageIndexMap;
     }
+
+    protected String getStorageCacheName() {
+        return cacheKey;
+    }
 }
diff --git a/src/main/java/org/commcare/cases/instance/LedgerInstanceTreeElement.java b/src/main/java/org/commcare/cases/instance/LedgerInstanceTreeElement.java
@@ -44,4 +44,8 @@ protected Hashtable<XPathPathExpr, String> getStorageIndexMap() {
 
         return indices;
     }
+
+    protected String getStorageCacheName() {
+        return MODEL_NAME;
+    }
 }
diff --git a/src/main/java/org/commcare/cases/instance/StorageInstanceTreeElement.java b/src/main/java/org/commcare/cases/instance/StorageInstanceTreeElement.java
@@ -2,6 +2,9 @@
 
 import org.commcare.cases.query.QueryContext;
 import org.commcare.cases.util.StorageBackedTreeRoot;
+import org.commcare.modern.engine.cases.RecordObjectCache;
+import org.commcare.modern.engine.cases.RecordSetResultCache;
+import org.commcare.modern.util.Pair;
 import org.javarosa.core.model.data.IAnswerData;
 import org.javarosa.core.model.instance.AbstractTreeElement;
 import org.javarosa.core.model.instance.TreeElement;
@@ -14,6 +17,7 @@
 import org.javarosa.core.util.Interner;
 import org.javarosa.core.util.externalizable.Externalizable;
 
+import java.util.LinkedHashSet;
 import java.util.Vector;
 
 /**
@@ -269,7 +273,60 @@ protected abstract T buildElement(StorageInstanceTreeElement<Model, T> storageIn
                                       int recordId, String id, int mult);
 
     protected Model getElement(int recordId, QueryContext context) {
-        EvaluationTrace trace = new EvaluationTrace("Model Load[" + childName+"]");
+        if (context == null || getStorageCacheName() == null) {
+            return getElementSingular(recordId, context);
+        }
+        RecordSetResultCache recordSetCache = context.getQueryCacheOrNull(RecordSetResultCache.class);
+
+        RecordObjectCache<Model> recordObjectCache = getRecordObjectCacheIfRelevant(context);
+
+        if(recordObjectCache != null) {
+            if (recordObjectCache.isLoaded(recordId)) {
+                return recordObjectCache.getLoadedRecordObject(recordId);
+            }
+
+            if (canLoadRecordFromGroup(recordSetCache, recordId)) {
+                Pair<String, LinkedHashSet<Integer>> tranche =
+                        recordSetCache.getRecordSetForRecordId(getStorageCacheName(), recordId);
+                EvaluationTrace loadTrace =
+                        new EvaluationTrace(String.format("Model [%s]: Bulk Load [%s}",
+                                this.getStorageCacheName(),tranche.first));
+
+                LinkedHashSet<Integer>  body = tranche.second;
+                storage.bulkRead(body, recordObjectCache.getLoadedCaseMap());
+                loadTrace.setOutcome("Loaded: " + body.size());
+                context.reportTrace(loadTrace);
+
+                return recordObjectCache.getLoadedRecordObject(recordId);
+            }
+        }
+
+        return getElementSingular(recordId, context);
+    }
+
+    private boolean canLoadRecordFromGroup(RecordSetResultCache recordSetCache, int recordId) {
+        return recordSetCache != null && recordSetCache.hasMatchingRecordSet(getStorageCacheName(), recordId);
+    }
+
+    /**
+     * Get a record object cache if it's appropriate in the current context.
+     */
+    private RecordObjectCache getRecordObjectCacheIfRelevant(QueryContext context) {
+        // If the query isn't currently in a bulk mode, don't force an object cache to exist unless
+        // it already does
+        if (context.getScope() < QueryContext.BULK_QUERY_THRESHOLD) {
+            return context.getQueryCacheOrNull(RecordObjectCache.class);
+        } else {
+            return context.getQueryCache(RecordObjectCache.class);
+        }
+    }
+
+    /**
+     * Retrieves a model for the provided record ID using a guaranteed singular lookup from
+     * storage. This is the "Safe" fallback behavior for lookups.
+     */
+    protected Model getElementSingular(int recordId, QueryContext context) {
+        EvaluationTrace trace = new EvaluationTrace(String.format("Model [%s]: Singular Load", getStorageCacheName()));
 
         Model m = storage.read(recordId);
 

diff --git a/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java b/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java
@@ -5,6 +5,8 @@
 import org.commcare.cases.query.IndexedValueLookup;
 import org.commcare.cases.query.PredicateProfile;
 import org.commcare.cases.query.handlers.BasicStorageBackedCachingQueryHandler;
+import org.commcare.modern.engine.cases.RecordSetResultCache;
+import org.commcare.modern.util.PerformanceTuningUtil;
 import org.javarosa.core.model.condition.EvaluationContext;
 import org.javarosa.core.model.instance.AbstractTreeElement;
 import org.javarosa.core.model.instance.TreeReference;
@@ -20,6 +22,7 @@
 import java.util.Collection;
 import java.util.Enumeration;
 import java.util.Hashtable;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Vector;
 
@@ -258,24 +261,39 @@ protected Collection<Integer> getNextIndexMatch(Vector<PredicateProfile> profile
 
         IndexedValueLookup op = (IndexedValueLookup)profiles.elementAt(0);
 
-
         EvaluationTrace trace = new EvaluationTrace("Model Index[" + op.key + "] Lookup");
 
         //Get matches if it works
-        List<Integer> returnValue = storage.getIDsForValue(op.key, op.value);
+        List<Integer> ids = storage.getIDsForValue(op.key, op.value);
+
+        if(getStorageCacheName() != null &&
+                ids.size() > 50 && ids.size() < PerformanceTuningUtil.getMaxPrefetchCaseBlock()) {
+            RecordSetResultCache cue = currentQueryContext.getQueryCache(RecordSetResultCache.class);
+            String bulkRecordSetKey = String.format("%s|%s", op.key, op.value);
+            cue.reportBulkRecordSet(bulkRecordSetKey, getStorageCacheName(), new LinkedHashSet(ids));
+        }
+
+
+        trace.setOutcome("results: " + ids.size());
 
-        trace.setOutcome("results: " + returnValue.size());
         if (currentQueryContext != null) {
             currentQueryContext.reportTrace(trace);
         }
 
         if(defaultCacher != null) {
-            defaultCacher.cacheResult(op.key, op.value, returnValue);
+            defaultCacher.cacheResult(op.key, op.value, ids);
         }
 
         //If we processed this, pop it off the queue
         profiles.removeElementAt(0);
 
-        return returnValue;
+        return ids;
     }
+
+    /**
+     * @return A string which will provide a unique name for the storage that is used in this tree
+     * root. Used to differentiate the record ID's retrieved during operations on this root in
+     * internal caches
+     */
+    protected abstract String getStorageCacheName();
 }
diff --git a/src/main/java/org/commcare/modern/engine/cases/CaseIndexQuerySetTransform.java b/src/main/java/org/commcare/modern/engine/cases/CaseIndexQuerySetTransform.java
@@ -1,5 +1,6 @@
 package org.commcare.modern.engine.cases;
 
+import org.commcare.cases.instance.CaseInstanceTreeElement;
 import org.commcare.cases.query.QueryContext;
 import org.commcare.cases.query.queryset.CaseQuerySetLookup;
 import org.commcare.cases.query.queryset.DerivedCaseQueryLookup;
@@ -68,7 +69,9 @@ protected ModelQuerySet loadModelQuerySet(QueryContext queryContext) {
         private void cacheCaseModelQuerySet(QueryContext queryContext, DualTableSingleMatchModelQuerySet ret) {
             int modelQueryMagnitude = ret.getSetBody().size();
             if(modelQueryMagnitude > QueryContext.BULK_QUERY_THRESHOLD && modelQueryMagnitude < PerformanceTuningUtil.getMaxPrefetchCaseBlock()) {
-                queryContext.getQueryCache(CaseSetResultCache.class).reportBulkCaseSet(this.getCurrentQuerySetId(), ret.getSetBody());
+                queryContext.getQueryCache(RecordSetResultCache.class).
+                        reportBulkRecordSet(this.getCurrentQuerySetId(),
+                                CaseInstanceTreeElement.MODEL_NAME, ret.getSetBody());
             }
         }
 

diff --git a/src/main/java/org/commcare/modern/engine/cases/CaseObjectCache.java b/src/main/java/org/commcare/modern/engine/cases/CaseObjectCache.java
diff --git a/src/main/java/org/commcare/modern/engine/cases/CaseSetResultCache.java b/src/main/java/org/commcare/modern/engine/cases/CaseSetResultCache.java
diff --git a/src/main/java/org/commcare/modern/engine/cases/RecordObjectCache.java b/src/main/java/org/commcare/modern/engine/cases/RecordObjectCache.java
@@ -0,0 +1,32 @@
+package org.commcare.modern.engine.cases;
+
+import org.commcare.cases.model.Case;
+import org.commcare.cases.query.QueryCache;
+
+import java.util.HashMap;
+
+/**
+ * A straightforward cache object query cache. Stores objects by their record ID.
+ *
+ * Used by other optimizations to isolate doing bulk loads and ensure that they are relevant
+ * when they occur
+ *
+ * Created by ctsims on 6/22/2017.
+ */
+
+public class RecordObjectCache<T> implements QueryCache {
+
+    private HashMap<Integer, T> cachedRecordObjects = new HashMap<>();
+
+    public boolean isLoaded(int recordId) {
+        return cachedRecordObjects.containsKey(recordId);
+    }
+
+    public HashMap<Integer, T> getLoadedCaseMap() {
+        return cachedRecordObjects;
+    }
+
+    public T getLoadedRecordObject(int recordId) {
+        return cachedRecordObjects.get(recordId);
+    }
+}
diff --git a/src/main/java/org/commcare/modern/engine/cases/RecordSetResultCache.java b/src/main/java/org/commcare/modern/engine/cases/RecordSetResultCache.java
@@ -0,0 +1,56 @@
+package org.commcare.modern.engine.cases;
+
+import org.commcare.cases.query.QueryCache;
+import org.commcare.modern.util.Pair;
+
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+
+/**
+ * A record set result cache keeps track of different sets of "Bulk" record which are
+ * likely to have data or operations tracked about them (IE: results of a common query which
+ * are likely to have further filtering applied.)
+ *
+ * Since these results are often captured/reported before a context is escalated, this cache
+ * doesn't directly hold the resulting cached records themselves. Rather a RecordObjectCache should
+ * be used to track the resulting records. This will ensure that cache can be attached to the
+ * appropriate lifecycle
+ *
+ * Created by ctsims on 1/25/2017.
+ */
+
+public class RecordSetResultCache implements QueryCache {
+
+    private HashMap<String,Pair<String, LinkedHashSet<Integer>>> bulkFetchBodies = new HashMap<>();
+
+    /**
+     * Report a set of bulk records that are likely to be needed as a group.
+     *
+     * @param key A unique key for the provided record set. It is presumed that if the key is
+     *            already in use that the id set is redundant.
+     * @param storageSetID The name of the Storage where the records are stored.
+     * @param ids The record set ID's
+     */
+    public void reportBulkRecordSet(String key, String storageSetID, LinkedHashSet<Integer> ids) {
+        String fullKey = key +"|" + storageSetID;
+        if (bulkFetchBodies.containsKey(fullKey)) {
+            return;
+        }
+        bulkFetchBodies.put(fullKey, new Pair<>(storageSetID, ids));
+    }
+
+    public boolean hasMatchingRecordSet(String recordSetName, int recordId) {
+        return getRecordSetForRecordId(recordSetName, recordId) != null;
+    }
+
+    public Pair<String, LinkedHashSet<Integer>> getRecordSetForRecordId(String recordSetName,
+                                                                        int recordId) {
+        for (String key : bulkFetchBodies.keySet()) {
+            Pair<String, LinkedHashSet<Integer>> tranche = bulkFetchBodies.get(key);
+            if (tranche.second.contains(recordId) && tranche.first.equals(recordSetName)) {
+                return new Pair<>(key, tranche.second);
+            }
+        }
+        return null;
+    }
+}