-
Notifications
You must be signed in to change notification settings - Fork 337
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.
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
Improve type inference performance to enable large queries #6431
Conversation
@@ -314,7 +316,7 @@ public void setSupertype(AttributeType superType) { | |||
|
|||
@Override | |||
public FunctionalIterator<AttributeTypeImpl> getSubtypes() { | |||
return getSubtypes(v -> { | |||
return graphMgr.schema().getSubtypes(vertex).map(v -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we move the method of getting subtypes from TypeImpl
to TypeGraph
so that the type graph doesn't have to repeat the exact code to get subtypes
@@ -141,10 +141,6 @@ private void validateTypeHierarchyIsNotCyclic() { | |||
} | |||
} | |||
|
|||
<TYPE extends Type> FunctionalIterator<TYPE> getSubtypes(Function<TypeVertex, TYPE> typeConstructor) { | |||
return tree(vertex, v -> v.ins().edge(SUB).from()).map(typeConstructor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functionality moved to TypeGraph
graph/TypeGraph.java
Outdated
private Set<Label> relations; | ||
private Set<Label> roles; | ||
private Set<Label> rolePlayers; | ||
private Set<Label> rolesPlayed; | ||
private Set<Label> attributes; | ||
private Set<Label> attributesOwned; | ||
private Set<Label> attributeOwners; | ||
private Set<Label> stringAttributes; | ||
private Set<Label> longAttributes; | ||
private Set<Label> doubleAttributes; | ||
private Set<Label> booleanAttributes; | ||
private Set<Label> datetimeAttributes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caches of sets of type labels we require for more efficient type resolution. We compute these over and over and they must be bound to the lifetime of the TypeGraph - so we reuse the Cache
object here to store them.
logic/LogicCache.java
Outdated
import java.util.Set; | ||
|
||
public class LogicCache { | ||
|
||
private CommonCache<Structure, Map<Identifier.Variable.Retrievable, Set<Label>>> typeResolverCache; | ||
private CommonCache<Structure, Optional<Map<Identifier.Variable.Retrievable, Set<Label>>>> typeResolverCache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use an optional instead of an empty map as a signal of an unsatisfiable query - this is more logical
logic/tool/TypeResolver.java
Outdated
} | ||
|
||
public FunctionalIterator<Map<Identifier.Variable.Name, Label>> namedCombinations(Conjunction conjunction, boolean insertable) { | ||
GraphTraversal resolverTraversal = new GraphTraversal(); | ||
TraversalBuilder traversalBuilder = new TraversalBuilder(conjunction, conceptMgr, resolverTraversal, 0, insertable); | ||
TypeTraversal resolverTraversal = new TypeTraversal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we created a new type of traversal just for traversing the type graph, called TypeTraversal
, which we use here
traversal/TraversalEngine.java
Outdated
public FunctionalIterator<VertexMap> iterator(GraphTraversal traversal) { | ||
return iterator(traversal, false); | ||
traversal.initialise(cache); | ||
return traversal.iterator(graphMgr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general Graph traversals must be query planned and cache the plan
traversal/TraversalEngine.java
Outdated
else traversal.initialise(cache); | ||
return traversal.iterator(graphMgr, singleUse); | ||
public FunctionalIterator<VertexMap> iterator(TypeTraversal traversal) { | ||
return traversal.iterator(graphMgr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type traversal plans are NOT cached
traversal/TraversalEngine.java
Outdated
public Optional<Map<Retrievable, Set<TypeVertex>>> combination(TypeTraversal traversal, | ||
Set<Retrievable> abstractDisallowed) { | ||
return new TypeCombination(graphMgr, TypeCombinationProcedure.of(traversal), traversal.parameters(), | ||
traversal.filter(), abstractDisallowed).get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and TypeTraversal
s can also be executed as a Combination, which presents an Optional API instead of an Iterator
traversal/Traversal.java
Outdated
@@ -60,6 +68,38 @@ public Parameters parameters() { | |||
return parameters; | |||
} | |||
|
|||
abstract FunctionalIterator<VertexMap> iterator(GraphManager graphMgr); | |||
|
|||
FunctionalIterator<VertexMap> iterator(GraphManager graphMgr, List<Planner> planners, boolean singleUse, Set<Identifier.Variable.Retrievable> filter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a common implementation for iterating over things that have Planner
objects - this is used by both GraphTraversal
and TypeTraversal
some of the TypeResolver tests pass write out progress and ideas
3 failing tests in type resolver
heuristically order combination exploration in increasing label size Don't re-explore types that have already been proven add remaining basic inferences cleanup
257dacd
to
e727cc9
Compare
40d5950
to
2dd948a
Compare
c8e3087
to
846e8a2
Compare
ae36be0
to
a3d771e
Compare
a3d771e
to
bdcc0f1
Compare
What is the goal of this PR?
Enable large queries to be processed by our type inference engine. Previously, large queries would cause the query to stall. A new type inference algorithm is implemented to massively speed up static type checking that is performed on every query.
What are the changes implemented in this PR?
GraphTraversal.Unrestricted
(previously justGraphTraversal
) andGraphTraversal.Type
, which is a graph traversal that can only operate over the schemaProcedure
toPermutationProcedure
, and createTypeCombinationProcedure
TypeCombinationGetter
, which executes aTypeCombinationProcedure
and returns an Optional with the type combination that was foundGraphProcedure.Builder
(intermediate state utilised it - we do not use it anymore)TraversalEngine
API