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

Jump to bottom

merge main into develop #5127

Merged
merged 5 commits into from
Sep 18, 2024
Merged

merge main into develop #5127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a0f374
wip
hmottestad Aug 29, 2024
10eb88e
wip
hmottestad Aug 29, 2024
89c9579
GH-5115 Fix for performance issues with ShaclSail after fix for compr…
hmottestad Sep 18, 2024
537ee67
Update merge_main_to_develop.yml
hmottestad Sep 18, 2024
a253fd5
Update merge_main_to_develop.yml (#5128)
hmottestad Sep 18, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/merge_main_to_develop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ jobs:
destination_branch: "develop" # If blank, default: main
pr_title: "Merge main into develop"
pr_body: "Automatically generated PR to keep develop in sync with main.\n\n **USE MERGE COMMIT TO MERGE THIS PR**.\n\nSee [merge_main_to_develop.yml](/eclipse/rdf4j/.github/workflows/merge_main_to_develop.yml)." # Full markdown support, requires pr_title to be set
github_token: ${{ secrets.GITHUB_TOKEN }}
github_token: ${{ github.token }}
5 changes: 3 additions & 2 deletions core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclSailConnection.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading