Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use lazy indexing repeated expression caching to optimize predicates #715

Merged
merged 47 commits into from
May 17, 2023
Merged

Use lazy indexing repeated expression caching to optimize predicates #715

Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
68799cb
Remove potentially unused code
seadowg Apr 11, 2023
5c35208
Extract predicate filtering to a method
seadowg Apr 11, 2023
d7c37b2
Use chained approach for caching implemenation
seadowg Apr 11, 2023
5b61f24
Simplify chain with recursive implementation
seadowg Apr 11, 2023
e88f90d
Allow chain to built up rather than set
seadowg Apr 11, 2023
beb837f
Use lambda instead of anonymous
seadowg Apr 11, 2023
a9cf53f
Implement limited between answer predicate caching
seadowg Apr 11, 2023
dbe239c
Add failing test for case where relative expression is on right hand …
seadowg Apr 11, 2023
b069f4c
Only allow expressions where the left side is relative
seadowg Apr 11, 2023
c8f0d1f
Rename param
seadowg Apr 12, 2023
a8dc7bd
Correct test form
seadowg Apr 12, 2023
890454d
Add stub for potentially failing case
seadowg Apr 12, 2023
8447ba3
Add failing test for cmatching predicates that come after different ones
seadowg Apr 13, 2023
5884aeb
Add check for potential regression
seadowg Apr 13, 2023
d50cbfc
Disable predicate caching for multiple predicates for the moment
seadowg Apr 13, 2023
de743ff
Add stub failing tests for other scenarios we want to make sure are o…
seadowg Apr 13, 2023
fbb0a14
Rewrite tests with Scenario DSL
seadowg Apr 13, 2023
4816c1b
Add passing test for comparison case
seadowg Apr 13, 2023
ab7226f
Add test for failing comp case
seadowg Apr 13, 2023
fc9b983
Support caching for cmp predicates
seadowg Apr 13, 2023
ae070b5
Add test for func case
seadowg Apr 17, 2023
b543f38
Evaluate eq predicates with index
seadowg Apr 17, 2023
3399e34
Split indexing caching out
seadowg Apr 17, 2023
aa9bcf2
Rename classes
seadowg Apr 17, 2023
e559d3e
Pull out expression parsing logic
seadowg Apr 17, 2023
6785e5a
Rename class
seadowg Apr 17, 2023
68c9667
Add failing tests for supporting more expressions
seadowg Apr 17, 2023
140f7cd
Support expressions regardless of direction
seadowg Apr 17, 2023
4fe85ae
Support numeric and string literals for index caching
seadowg Apr 17, 2023
a0c4568
Don't try and index nested nodesets
seadowg Apr 17, 2023
e8cdc1b
Rename class
seadowg Apr 19, 2023
aa21452
Move evaluation logic into class
seadowg May 4, 2023
59cb3f9
Fix different types of eq being confused
seadowg May 5, 2023
2bb898f
Prefer path expressions in regression tests where it's possible
seadowg May 5, 2023
fa1a824
Correct name of test
seadowg May 5, 2023
2753251
Don't apply caching to expressions on main instance
seadowg May 9, 2023
7ded84f
Add caching for first predicate in chain
seadowg May 9, 2023
b496772
Pull indexing data structure code out of predicate filter
seadowg May 9, 2023
3cc5c76
Correct Measure implementation
seadowg May 10, 2023
adc1463
Remove unused method
seadowg May 10, 2023
31fbd2f
Nest interface
seadowg May 12, 2023
fcf5703
Create default filter chain
seadowg May 12, 2023
a30fade
Remove interface that's only used in one class
seadowg May 12, 2023
44d944a
Add ability to add predicate filter
seadowg May 12, 2023
93a6d6c
Improve interface for clients
seadowg May 12, 2023
2dc034d
Move nesting check to EvaluationContext
seadowg May 12, 2023
f08c2b3
Add test to check we aren't increasing load time when indexing
seadowg May 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/main/java/org/javarosa/core/model/condition/EvaluationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,16 @@ private void expandReferenceAccumulator(TreeReference sourceRef, DataInstance so
TreeReference nodeSetRef = workingRef.clone();
nodeSetRef.add(name, -1);

boolean firstTime = true;
List<TreeReference> passed = new ArrayList<TreeReference>(treeReferences.size());
for (XPathExpression xpe : predicates) {
boolean firstTimeCapture = firstTime;
passed.addAll(predicateCache.get(nodeSetRef, xpe, () -> {
List<TreeReference> predicatePassed = new ArrayList<>(treeReferences.size());
for (int i = 0; i < treeReferences.size(); ++i) {
//if there are predicates then we need to see if e.nextElement meets the standard of the predicate
TreeReference treeRef = treeReferences.get(i);

//test the predicate on the treeElement
EvaluationContext evalContext = rescope(treeRef, (firstTimeCapture ? treeRef.getMultLast() : i));
EvaluationContext evalContext = rescope(treeRef, i);
Copy link
Member

@lognaturel lognaturel Jun 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble convincing myself that the whole condition is ok to delete. Can you explain the reasoning please? Or is it just that there are no tests for it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It had no tests and I couldn't explain the reasoning for it. Agreed it feels scary to delete, but it's also completely unexplained code. Do you feel like we need to add the logic back in? I couldn't come up with an example at the time, but I was heavily down in the weeds. Maybe we'd have more success reverse engineering a test now.


Measure.log("PredicateEvaluations");
Object o = xpe.eval(sourceInstance, evalContext);
Expand All @@ -352,7 +350,6 @@ private void expandReferenceAccumulator(TreeReference sourceRef, DataInstance so
return predicatePassed;
}));

firstTime = false;
treeReferences.clear();
treeReferences.addAll(passed);
passed.clear();
Expand Down