Merge pull request #742 from seadowg/caching

Add caching for and/or and bring ItemsetBinding caching back

src/main/java/org/javarosa/core/model/ComparisonExpressionCacheFilterStrategy.java

@@ -4,6 +4,7 @@
 import org.javarosa.core.model.condition.FilterStrategy;
 import org.javarosa.core.model.instance.DataInstance;
 import org.javarosa.core.model.instance.TreeReference;
+import org.javarosa.xpath.expr.XPathBoolExpr;
 import org.javarosa.xpath.expr.XPathCmpExpr;
 import org.javarosa.xpath.expr.XPathEqExpr;
 import org.javarosa.xpath.expr.XPathExpression;
@@ -31,19 +32,43 @@ public List<TreeReference> filter(@NotNull DataInstance sourceInstance, @NotNull
 
  CompareToNodeExpression candidate = CompareToNodeExpression.parse(predicate);
  if (candidate != null) {
- Object absoluteValue = candidate.evalContextSide(sourceInstance, evaluationContext);
- String key = nodeSet.toString() + predicate + candidate.getNodeSide() + absoluteValue.toString();
+ String key = getExpressionKey(sourceInstance, nodeSet, predicate, evaluationContext, candidate);
 
- if (cachedEvaluations.containsKey(key)) {
- return cachedEvaluations.get(key);
+ return getCachedEvaluations(next, key);
+ } else if (predicate instanceof XPathBoolExpr) {
+ XPathExpression a = ((XPathBoolExpr) predicate).a;
+ XPathExpression b = ((XPathBoolExpr) predicate).b;
+
+ CompareToNodeExpression candidateA = CompareToNodeExpression.parse(a);
+ CompareToNodeExpression candidateB = CompareToNodeExpression.parse(b);
+
+ if (candidateA != null && candidateB != null) {
+ String keyA = getExpressionKey(sourceInstance, nodeSet, a, evaluationContext, candidateA);
+ String keyB = getExpressionKey(sourceInstance, nodeSet, b, evaluationContext, candidateB);
+ String key = "XPathBoolExpr:" + ((XPathBoolExpr) predicate).op + keyA + keyB;
+
+ return getCachedEvaluations(next, key);
  } else {
- List<TreeReference> filtered = next.get();
- cachedEvaluations.put(key, filtered);
- return filtered;
+ return next.get();
  }
  } else {
  return next.get();
  }
  }
 
+ private List<TreeReference> getCachedEvaluations(@NotNull Supplier<List<TreeReference>> next, String key) {
+ if (cachedEvaluations.containsKey(key)) {
+ return cachedEvaluations.get(key);
+ } else {
+ List<TreeReference> filtered = next.get();
+ cachedEvaluations.put(key, filtered);
+ return filtered;
+ }
+ }
+
+ @NotNull
+ private static String getExpressionKey(@NotNull DataInstance sourceInstance, @NotNull TreeReference nodeSet, @NotNull XPathExpression predicate, @NotNull EvaluationContext evaluationContext, CompareToNodeExpression candidate) {
+ Object absoluteValue = candidate.evalContextSide(sourceInstance, evaluationContext);
+ return nodeSet.toString() + predicate + candidate.getNodeSide() + absoluteValue.toString();
+ }
 }

src/main/java/org/javarosa/core/model/ItemsetBinding.java

@@ -11,6 +11,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import org.javarosa.core.model.condition.EvaluationContext;
 import org.javarosa.core.model.condition.IConditionExpr;
 import org.javarosa.core.model.data.IAnswerData;
@@ -38,6 +40,12 @@
 import org.javarosa.xpath.expr.XPathPathExpr;
 
 public class ItemsetBinding implements Externalizable, Localizable {
+ // Temporarily cached filtered list (not serialized)
+ private List<SelectChoice> cachedFilteredChoiceList;
+
+ // Values needed to determine whether the cached list should be expired (not serialized)
+ private Map<TreeReference, IAnswerData> cachedTriggerValues;
+ private Long cachedRandomizeSeed;
 
  /**
  * note that storing both the ref and expr for everything is kind of redundant, but we're forced
@@ -47,8 +55,8 @@ public class ItemsetBinding implements Externalizable, Localizable {
  public TreeReference nodesetRef; //absolute ref of itemset source nodes
  public IConditionExpr nodesetExpr; //path expression for source nodes; may be relative, may contain predicates
  public TreeReference contextRef; //context ref for nodesetExpr; ref of the control parent (group/formdef) of itemset question
-  //note: this is only here because it's currently impossible to both (a) get a form control's parent, and (b)
-  //convert expressions into refs while preserving predicates. once these are fixed, this field can go away
+ //note: this is only here because it's currently impossible to both (a) get a form control's parent, and (b)
+ //convert expressions into refs while preserving predicates. once these are fixed, this field can go away
 
  public TreeReference labelRef; //absolute ref of label
  public IConditionExpr labelExpr; //path expression for label; may be relative, no predicates
@@ -58,7 +66,7 @@ public class ItemsetBinding implements Externalizable, Localizable {
  public IConditionExpr valueExpr; //path expression for value; may be relative, no predicates (must be relative if copy mode)
 
  private TreeReference destRef; //ref that identifies the repeated nodes resulting from this itemset
-  //not serialized -- set by QuestionDef.setDynamicChoices()
+ //not serialized -- set by QuestionDef.setDynamicChoices()
 
  public boolean randomize = false;
  public XPathNumericLiteral randomSeedNumericExpr = null;
@@ -89,8 +97,19 @@ public class ItemsetBinding implements Externalizable, Localizable {
  * part of the new filtered list is removed and the new answer is saved back to the model.
  */
  public List<SelectChoice> getChoices(FormDef formDef, TreeReference curQRef) {
+ Map<TreeReference, IAnswerData> currentTriggerValues = getCurrentTriggerValues(formDef, curQRef);
+ boolean allTriggerRefsBound = currentTriggerValues != null;
+
  Long currentRandomizeSeed = resolveRandomSeed(formDef.getMainInstance(), formDef.getEvaluationContext());
 
+ // Return cached list if possible
+ if (cachedFilteredChoiceList != null && allTriggerRefsBound && Objects.equals(currentTriggerValues, cachedTriggerValues)
+ && Objects.equals(currentRandomizeSeed, cachedRandomizeSeed)) {
+ updateQuestionAnswerInModel(formDef, curQRef);
+
+ return randomize && cachedRandomizeSeed == null ? shuffle(cachedFilteredChoiceList) : cachedFilteredChoiceList;
+ }
+
  formDef.getEventNotifier().publishEvent(new Event("Dynamic choices", new EvaluationResult(curQRef, null)));
 
  DataInstance formInstance;
@@ -103,8 +122,8 @@ public List<SelectChoice> getChoices(FormDef formDef, TreeReference curQRef) {
  formInstance = formDef.getMainInstance();
  }
 
- EvaluationContext evalContext = new EvaluationContext(formDef.getEvaluationContext(), contextRef.contextualize(curQRef));
- List<TreeReference> filteredItemReferences = nodesetExpr.evalNodeset(formDef.getMainInstance(), evalContext);
+ List<TreeReference> filteredItemReferences = nodesetExpr.evalNodeset(formDef.getMainInstance(),
+  new EvaluationContext(formDef.getEvaluationContext(), contextRef.contextualize(curQRef)));
 
  if (filteredItemReferences == null) {
  throw new XPathException("Could not find references depended on by" + nodesetRef.getInstanceName());
@@ -124,6 +143,8 @@ public List<SelectChoice> getChoices(FormDef formDef, TreeReference curQRef) {
 
  updateQuestionAnswerInModel(formDef, curQRef, selectChoicesForAnswer);
 
+ cachedFilteredChoiceList = randomize ? shuffle(filteredChoiceList, currentRandomizeSeed) : filteredChoiceList;
+
  // TODO: write a test that fails if this is removed. It looks like a no-op because it's not accessing the shuffled collection.
  if (randomize) {
  // Match indices to new positions
@@ -140,7 +161,38 @@ public List<SelectChoice> getChoices(FormDef formDef, TreeReference curQRef) {
  }
  }
 
- return randomize ? shuffle(filteredChoiceList, currentRandomizeSeed) : filteredChoiceList;
+ cachedTriggerValues = currentTriggerValues;
+ cachedRandomizeSeed = currentRandomizeSeed;
+
+ return cachedFilteredChoiceList;
+ }
+
+ /**
+ * Returns a map:
+ * - keys: the references that are triggers for the nodeset expression
+ * - values: current values at those references
+ *
+ * Returns null if the nodeset expression has any triggers that are unbounded references because there's no single
+ * value we could track in that case.
+ */
+ private Map<TreeReference, IAnswerData> getCurrentTriggerValues(FormDef formDef, TreeReference curQRef) {
+ Map<TreeReference, IAnswerData> currentTriggerValues = new HashMap<>();
+
+ Set<TreeReference> triggers = nodesetExpr.getTriggers(curQRef);
+ for (TreeReference trigger : triggers) {
+ // Only store values for expressions in the primary instance. Secondary instances can never change so no need to store their values.
+ if (trigger.getInstanceName() == null) {
+ TreeElement element = formDef.getMainInstance().resolveReference(trigger);
+
+ // Unbounded references (e.g. ref to a repeat nodeset rather than a repeat instance) don't have a value we can keep track of.
+ if (element != null && !element.isRepeatable()) {
+ currentTriggerValues.put(trigger, element.getValue());
+ } else {
+ return null;
+ }
+ }
+ }
+ return currentTriggerValues;
  }
 
  private SelectChoice getChoiceForTreeReference(FormDef formDef, DataInstance formInstance, int i, TreeReference item) {
@@ -219,6 +271,23 @@ private void updateQuestionAnswerInModel(FormDef formDef, TreeReference curQRef,
  }
  }
 
+ /**
+ * Updates the model using the previously-cached choice list.
+ *
+ * @see #updateQuestionAnswerInModel(FormDef, TreeReference, Map) for details and side-effects
+ */
+ private void updateQuestionAnswerInModel(FormDef formDef, TreeReference curQRef) {
+ Map<String, SelectChoice> selectChoicesForAnswer = initializeAnswerMap(formDef, curQRef);
+ if (selectChoicesForAnswer != null) {
+ for (SelectChoice choice : cachedFilteredChoiceList) {
+ if (selectChoicesForAnswer.containsKey(choice.getValue())) {
+ selectChoicesForAnswer.put(choice.getValue(), choice);
+ }
+ }
+ }
+ updateQuestionAnswerInModel(formDef, curQRef, selectChoicesForAnswer);
+ }
+
  /**
  * @param selections an answer to a multiple selection question
  * @param selectChoicesForAnswer maps each value that could be in @{code selections} to a SelectChoice if it should be bound
@@ -247,9 +316,12 @@ private Long resolveRandomSeed(DataInstance model, EvaluationContext ec) {
  return null;
  }
 
- @Override
  public void localeChanged(String locale, Localizer localizer) {
-
+ if (cachedFilteredChoiceList != null) {
+ for (SelectChoice selectChoice : cachedFilteredChoiceList) {
+ selectChoice.localeChanged(locale, localizer);
+ }
+ }
  }
 
  public TreeReference getDestRef () {
@@ -323,5 +395,4 @@ public void writeExternal(DataOutputStream out) throws IOException {
  ExtUtil.write(out, new ExtWrapNullable(randomSeedNumericExpr == null ? null : new ExtWrapTagged(randomSeedNumericExpr)));
  ExtUtil.write(out, new ExtWrapNullable(randomSeedPathExpr == null ? null : new ExtWrapTagged(randomSeedPathExpr)));
  }
-
-}
+}