Skip to content

Commit

Permalink
GH-5115 Fix for performance issues with ShaclSail after fix for compr…
Browse files Browse the repository at this point in the history 
…essed validation tuples (#5119)
  • Loading branch information
@hmottestad
hmottestad authored Sep 18, 2024
2 parents 5c956e1 + 10eb88e commit 89c9579
Show file tree
Hide file tree
Showing 99 changed files with 3,861 additions and 542 deletions.
5 changes: 3 additions & 2 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclSailConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,9 @@ private ValidationReport performValidation(List<ContextWithShape> shapes, boolea
sail.isPerformanceLogging())),
sail.isGlobalLogValidationExecution(), sail.isLogValidationViolations(),
sail.getEffectiveValidationResultsLimitPerConstraint(), sail.isPerformanceLogging(),
logger
))
sail.isLogValidationPlans(),
logger,
connectionsGroup))

.filter(ShapeValidationContainer::hasPlanNode)
.map(validationContainer -> validationContainer::performValidation);
Expand Down
4 changes: 2 additions & 2 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ private static ValidationReport performValidation(List<ContextWithShape> shapes,
() -> contextWithShape.getShape()
.generatePlans(connectionsGroup,
new ValidationSettings(contextWithShape.getDataGraph(), false, true, false)),
false, false, 1000, false, logger
)
false, false, 1000, false, false, logger,
connectionsGroup)
)
.filter(ShapeValidationContainer::hasPlanNode)
.map(ShapeValidationContainer::performValidation)
Expand Down
57 changes: 55 additions & 2 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShapeValidationContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,22 @@

package org.eclipse.rdf4j.sail.shacl;

import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.apache.commons.text.StringEscapeUtils;
import org.eclipse.rdf4j.common.iteration.CloseableIteration;
import org.eclipse.rdf4j.sail.SailConnection;
import org.eclipse.rdf4j.sail.SailException;
import org.eclipse.rdf4j.sail.shacl.ast.Shape;
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.SingleCloseablePlanNode;
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.ValidationExecutionLogger;
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.ValidationTuple;
import org.eclipse.rdf4j.sail.shacl.results.lazy.ValidationResultIterator;
import org.eclipse.rdf4j.sail.shacl.wrapper.data.ConnectionsGroup;
import org.slf4j.Logger;

class ShapeValidationContainer {
Expand All @@ -36,14 +40,64 @@ class ShapeValidationContainer {

public ShapeValidationContainer(Shape shape, Supplier<PlanNode> planNodeSupplier, boolean logValidationExecution,
boolean logValidationViolations, long effectiveValidationResultsLimitPerConstraint,
boolean performanceLogging, Logger logger) {
boolean performanceLogging, boolean logValidationPlans, Logger logger, ConnectionsGroup connectionsGroup) {
this.shape = shape;
this.logValidationViolations = logValidationViolations;
this.effectiveValidationResultsLimitPerConstraint = effectiveValidationResultsLimitPerConstraint;
this.performanceLogging = performanceLogging;
this.logger = logger;
try {
PlanNode planNode = planNodeSupplier.get();

if (logValidationPlans) {

StringBuilder planAsGraphvizDot = new StringBuilder();

planAsGraphvizDot.append(
"rank1 [style=invisible];\n" +
"rank2 [style=invisible];\n" +
"\n" +
"rank1 -> rank2 [color=white];\n");

planAsGraphvizDot.append("{\n")
.append("\trank = same;\n")
.append("\trank2 -> ")
.append(System.identityHashCode(connectionsGroup.getBaseConnection()))
.append(" -> ")
.append(System.identityHashCode(connectionsGroup.getAddedStatements()))
.append(" -> ")
.append(System.identityHashCode(connectionsGroup.getRemovedStatements()))
.append(" [ style=invis ];\n")
.append("\trankdir = LR;\n")
.append("}\n");

planAsGraphvizDot.append(System.identityHashCode(connectionsGroup.getBaseConnection()))
.append(" [label=\"")
.append("BaseConnection")
.append("\" fillcolor=\"#CACADB\", style=filled];")
.append("\n");

planAsGraphvizDot.append(System.identityHashCode(connectionsGroup.getAddedStatements()))
.append(" [label=\"")
.append("AddedStatements")
.append("\" fillcolor=\"#CEDBCA\", style=filled];")
.append("\n");

planAsGraphvizDot.append(System.identityHashCode(connectionsGroup.getRemovedStatements()))
.append(" [label=\"")
.append("RemovedStatements")
.append("\" fillcolor=\"#DBCFC9r\", style=filled];")
.append("\n");

planNode.getPlanAsGraphvizDot(planAsGraphvizDot);

String[] split = planAsGraphvizDot.toString().split("\n");
planAsGraphvizDot = new StringBuilder();
Arrays.stream(split).map(s -> "\t" + s + "\n").forEach(planAsGraphvizDot::append);

logger.info("Plan as Graphviz dot:\ndigraph G {\n{}}", planAsGraphvizDot);
}

this.validationExecutionLogger = ValidationExecutionLogger
.getInstance(logValidationExecution);
if (!(planNode.isGuaranteedEmpty())) {
Expand Down Expand Up @@ -113,7 +167,6 @@ private void handlePostLogging(long before, ValidationResultIterator validationR
}

if (validationResults != null) {

if (performanceLogging) {
long after = System.currentTimeMillis();
logger.info("Execution of plan took {} ms for:\n{}\n",
Expand Down
15 changes: 8 additions & 7 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/NodeShape.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,15 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
return new ValidationResult(t.getActiveTarget(), t.getActiveTarget(), this,
constraintComponent, getSeverity(), t.getScope(), t.getContexts(),
getContexts());
});
}, connectionsGroup);
}

if (scope == Scope.propertyShape) {
validationPlanNode = Unique.getInstance(new ShiftToPropertyShape(validationPlanNode), true);
validationPlanNode = Unique.getInstance(new ShiftToPropertyShape(validationPlanNode, connectionsGroup),
true, connectionsGroup);
}

union = UnionNode.getInstance(union, validationPlanNode);
union = UnionNode.getInstance(connectionsGroup, union, validationPlanNode);
}

return union;
Expand All @@ -207,7 +208,7 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
.map(c -> c.getAllTargetsPlan(connectionsGroup, dataGraph, Scope.nodeShape,
new StatementMatcher.StableRandomVariableProvider()))
.distinct()
.reduce(UnionNode::getInstanceDedupe)
.reduce((nodes, nodes2) -> UnionNode.getInstanceDedupe(connectionsGroup, nodes, nodes2))
.orElse(EmptyNode.getInstance());

if (connectionsGroup.getStats().hasRemoved()) {
Expand All @@ -216,14 +217,14 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
stableRandomVariableProvider)
.getPlanNode(connectionsGroup, dataGraph, Scope.nodeShape, true, null);

planNode = UnionNode.getInstanceDedupe(planNode, planNodeEffectiveTarget);
planNode = UnionNode.getInstanceDedupe(connectionsGroup, planNode, planNodeEffectiveTarget);
}

if (scope == Scope.propertyShape) {
planNode = Unique.getInstance(new ShiftToPropertyShape(planNode), true);
planNode = Unique.getInstance(new ShiftToPropertyShape(planNode, connectionsGroup), true, connectionsGroup);
}

planNode = Unique.getInstance(planNode, false);
planNode = Unique.getInstance(planNode, false, connectionsGroup);

return planNode;
}
Expand Down
20 changes: 11 additions & 9 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/PropertyShape.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,16 +236,18 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
return new ValidationResult(t.getActiveTarget(), t.getValue(), this,
constraintComponent, getSeverity(), t.getScope(), t.getContexts(),
getContexts());
});
}, connectionsGroup);
}

if (scope == Scope.propertyShape) {
validationPlanNode = Unique.getInstance(new TargetChainPopper(validationPlanNode), true);
validationPlanNode = Unique.getInstance(new TargetChainPopper(validationPlanNode, connectionsGroup),
true, connectionsGroup);
} else {
validationPlanNode = Unique.getInstance(new ShiftToNodeShape(validationPlanNode), true);
validationPlanNode = Unique.getInstance(new ShiftToNodeShape(validationPlanNode, connectionsGroup),
true, connectionsGroup);
}

union = UnionNode.getInstance(union, validationPlanNode);
union = UnionNode.getInstance(connectionsGroup, union, validationPlanNode);
}

return union;
Expand All @@ -258,7 +260,7 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
.map(c -> c.getAllTargetsPlan(connectionsGroup, dataGraph, Scope.propertyShape,
new StatementMatcher.StableRandomVariableProvider()))
.distinct()
.reduce(UnionNode::getInstanceDedupe)
.reduce((nodes, nodes2) -> UnionNode.getInstanceDedupe(connectionsGroup, nodes, nodes2))
.orElse(EmptyNode.getInstance());

if (connectionsGroup.getStats().hasRemoved()) {
Expand All @@ -267,16 +269,16 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
stableRandomVariableProvider)
.getPlanNode(connectionsGroup, dataGraph, Scope.propertyShape, true, null);

planNode = UnionNode.getInstanceDedupe(planNode, planNodeEffectiveTarget);
planNode = UnionNode.getInstanceDedupe(connectionsGroup, planNode, planNodeEffectiveTarget);
}

if (scope == Scope.propertyShape) {
planNode = Unique.getInstance(new TargetChainPopper(planNode), true);
planNode = Unique.getInstance(new TargetChainPopper(planNode, connectionsGroup), true, connectionsGroup);
} else {
planNode = new ShiftToNodeShape(planNode);
planNode = new ShiftToNodeShape(planNode, connectionsGroup);
}

planNode = Unique.getInstance(planNode, false);
planNode = Unique.getInstance(planNode, false, connectionsGroup);

return planNode;
}
Expand Down
6 changes: 3 additions & 3 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/Shape.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ public PlanNode generatePlans(ConnectionsGroup connectionsGroup, ValidationSetti
Scope.none)
.getValidationPlan(connectionsGroup.getBaseConnection(), validationSettings.getDataGraph(),
getContexts()),
this);
this, connectionsGroup);
} else {
logger.debug("Use fall back validation approach for bulk validation instead of SPARQL for shape {}",
this);
Expand All @@ -436,7 +436,7 @@ public PlanNode generatePlans(ConnectionsGroup connectionsGroup, ValidationSetti
.getAllTargets(connectionsGroup,
validationSettings.getDataGraph(),
this instanceof NodeShape ? Scope.nodeShape : Scope.propertyShape),
Scope.none), this);
Scope.none), this, connectionsGroup);
}

} else if (validationApproach == ValidationApproach.Transactional) {
Expand All @@ -447,7 +447,7 @@ public PlanNode generatePlans(ConnectionsGroup connectionsGroup, ValidationSetti
return new SingleCloseablePlanNode(
Shape.this.generateTransactionalValidationPlan(connectionsGroup, validationSettings, null,
Scope.none),
this);
this, connectionsGroup);
} else {
return EmptyNode.getInstance();
}
Expand Down
8 changes: 4 additions & 4 deletions ...va/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractConstraintComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,22 @@ static PlanNode getAllTargetsIncludingThoseAddedByPath(ConnectionsGroup connecti
ValidationSettings validationSettings, Scope scope, EffectiveTarget effectiveTarget, Path path,
boolean includeTargetsAffectedByRemoval) {
PlanNode allTargets;
BufferedSplitter addedTargets = new BufferedSplitter(
BufferedSplitter addedTargets = BufferedSplitter.getInstance(
effectiveTarget.getPlanNode(connectionsGroup, validationSettings.getDataGraph(),
scope, includeTargetsAffectedByRemoval, null));

PlanNode addedByPath = path.getAllAdded(connectionsGroup, validationSettings.getDataGraph(), null);

addedByPath = Unique.getInstance(new TrimToTarget(addedByPath), false);
addedByPath = Unique.getInstance(new TrimToTarget(addedByPath, connectionsGroup), false, connectionsGroup);

addedByPath = new ReduceTargets(addedByPath, addedTargets.getPlanNode());
addedByPath = new ReduceTargets(addedByPath, addedTargets.getPlanNode(), connectionsGroup);

addedByPath = effectiveTarget.extend(addedByPath, connectionsGroup, validationSettings.getDataGraph(),
scope, EffectiveTarget.Extend.left,
false,
null);

allTargets = UnionNode.getInstance(addedTargets.getPlanNode(), addedByPath);
allTargets = UnionNode.getInstance(connectionsGroup, addedTargets.getPlanNode(), addedByPath);
return allTargets;
}

Expand Down
25 changes: 15 additions & 10 deletions ...clipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractPairwiseConstraintComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
validationSettings,
effectiveTarget);

allTargets = Unique.getInstance(UnionNode.getInstance(allTargets, allTargetsBasedOnPredicate), false);
allTargets = Unique.getInstance(
UnionNode.getInstance(connectionsGroup, allTargets, allTargetsBasedOnPredicate), false,
connectionsGroup);

} else {
allTargets = effectiveTarget.getPlanNode(connectionsGroup, validationSettings.getDataGraph(), scope,
Expand All @@ -99,7 +101,9 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
validationSettings,
effectiveTarget);

allTargets = Unique.getInstance(UnionNode.getInstance(allTargets, allTargetsBasedOnPredicate), false);
allTargets = Unique.getInstance(
UnionNode.getInstance(connectionsGroup, allTargets, allTargetsBasedOnPredicate), false,
connectionsGroup);
}

}
Expand Down Expand Up @@ -151,7 +155,8 @@ private PlanNode getAllTargetsBasedOnPredicate(ConnectionsGroup connectionsGroup
PlanNode targetFilter2 = effectiveTarget.getTargetFilter(connectionsGroup, validationSettings.getDataGraph(),
removedByPredicate);

return Unique.getInstance(UnionNode.getInstance(targetFilter1, targetFilter2), false);
return Unique.getInstance(UnionNode.getInstance(connectionsGroup, targetFilter1, targetFilter2), false,
connectionsGroup);
}

@Override
Expand All @@ -163,7 +168,7 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
stableRandomVariableProvider)
.getPlanNode(connectionsGroup, dataGraph, Scope.nodeShape, true, null);

allTargetsPlan = new ShiftToPropertyShape(allTargetsPlan);
allTargetsPlan = new ShiftToPropertyShape(allTargetsPlan, connectionsGroup);

// removed statements that match predicate could affect sh:or
if (connectionsGroup.getStats().hasRemoved()) {
Expand All @@ -181,7 +186,7 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
EffectiveTarget.Extend.left,
false,
null);
allTargetsPlan = UnionNode.getInstanceDedupe(allTargetsPlan, deletedPredicates);
allTargetsPlan = UnionNode.getInstanceDedupe(connectionsGroup, allTargetsPlan, deletedPredicates);
}

// added statements that match predicate could affect sh:not
Expand All @@ -200,10 +205,10 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
EffectiveTarget.Extend.left,
false,
null);
allTargetsPlan = UnionNode.getInstanceDedupe(allTargetsPlan, addedPredicates);
allTargetsPlan = UnionNode.getInstanceDedupe(connectionsGroup, allTargetsPlan, addedPredicates);
}

return Unique.getInstance(new TrimToTarget(allTargetsPlan), false);
return Unique.getInstance(new TrimToTarget(allTargetsPlan, connectionsGroup), false, connectionsGroup);
} else {
assert scope == Scope.nodeShape;

Expand All @@ -224,7 +229,7 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
.extend(deletedPredicates, connectionsGroup, dataGraph, Scope.nodeShape,
EffectiveTarget.Extend.left,
false, null);
allTargetsPlan = UnionNode.getInstanceDedupe(allTargetsPlan, deletedPredicates);
allTargetsPlan = UnionNode.getInstanceDedupe(connectionsGroup, allTargetsPlan, deletedPredicates);

}

Expand All @@ -243,11 +248,11 @@ public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[]
.extend(addedPredicates, connectionsGroup, dataGraph, Scope.nodeShape,
EffectiveTarget.Extend.left,
false, null);
allTargetsPlan = UnionNode.getInstanceDedupe(allTargetsPlan, addedPredicates);
allTargetsPlan = UnionNode.getInstanceDedupe(connectionsGroup, allTargetsPlan, addedPredicates);

}

return Unique.getInstance(allTargetsPlan, false);
return Unique.getInstance(allTargetsPlan, false, connectionsGroup);

}

Expand Down
Loading

0 comments on commit 89c9579

Please sign in to comment.