Skip to content

Commit 3a8ecbf

Browse files
committed
GH-2782 - Optimize relationship processing.
For a lot of relationships on a self-referencing type (10k+), the result will be in the format `n, collect(rel), collect(relNode)`. This brings the whole bucket of `relNodes` every time as a mappable option to the table. Prior to this change, SDN would have to check this complete bucket for potential matching nodes. Just to find out later that there is no relationship to map this one for. Checking the relationship bucket first to find if there is a relationship at all to find a target node for, improves the performance drastically. Closes #2782
1 parent baed5f1 commit 3a8ecbf

File tree

1 file changed

+58
-55
lines changed

1 file changed

+58
-55
lines changed

src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java

+58-55
Original file line numberDiff line numberDiff line change
@@ -662,86 +662,89 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
662662

663663
String elementId = IdentitySupport.getElementId(values);
664664
Long internalId = IdentitySupport.getInternalId(values);
665-
boolean hasGeneratedIdValue = elementId != null || internalId != null;
665+
boolean hasIdValue = elementId != null || internalId != null;
666666

667-
if (relationshipListEmptyOrNull && hasGeneratedIdValue) {
667+
if (relationshipListEmptyOrNull && hasIdValue) {
668668
String sourceNodeId;
669669
Function<Relationship, String> sourceIdSelector;
670670
Function<Relationship, String> targetIdSelector = relationshipDescription.isIncoming() ? Relationship::startNodeElementId : Relationship::endNodeElementId;
671671

672-
if (internalId != null) {
672+
if (elementId != null) {
673+
sourceNodeId = elementId;
674+
sourceIdSelector = relationshipDescription.isIncoming() ? Relationship::endNodeElementId : Relationship::startNodeElementId;
675+
} else {
673676
// this can happen when someone used dto mapping and added the "classical" approach
674677
sourceNodeId = Long.toString(internalId);
675678
Function<Relationship, Long> hlp = relationshipDescription.isIncoming() ? Relationship::endNodeId : Relationship::startNodeId;
676679
sourceIdSelector = hlp.andThen(l -> Long.toString(l));
677-
} else {
678-
sourceNodeId = elementId;
679-
sourceIdSelector = relationshipDescription.isIncoming() ? Relationship::endNodeElementId : Relationship::startNodeElementId;
680680
}
681681

682682
// Retrieve all matching relationships from the result's list(s)
683683
Collection<Relationship> allMatchingTypeRelationshipsInResult =
684684
extractMatchingRelationships(relationshipsFromResult, relationshipDescription, typeOfRelationship,
685685
(possibleRelationship) -> sourceIdSelector.apply(possibleRelationship).equals(sourceNodeId));
686686

687-
// Retrieve all nodes from the result's list(s)
688-
Collection<Node> allNodesWithMatchingLabelInResult = extractMatchingNodes(nodesFromResult, targetLabel);
687+
// Fast exit if there is no relationship that can be mapped
688+
if (!allMatchingTypeRelationshipsInResult.isEmpty()) {
689689

690-
for (Node possibleValueNode : allNodesWithMatchingLabelInResult) {
691-
String targetNodeId = IdentitySupport.getElementId(possibleValueNode);
690+
// Retrieve all nodes from the result's list(s)
691+
Collection<Node> allNodesWithMatchingLabelInResult = extractMatchingNodes(nodesFromResult, targetLabel);
692+
for (Node possibleValueNode : allNodesWithMatchingLabelInResult) {
693+
String targetNodeId = IdentitySupport.getElementId(possibleValueNode);
692694

693-
Neo4jPersistentEntity<?> concreteTargetNodeDescription =
694-
getMostConcreteTargetNodeDescription(genericTargetNodeDescription, possibleValueNode);
695-
696-
Set<Relationship> relationshipsProcessed = new HashSet<>();
697-
for (Relationship possibleRelationship : allMatchingTypeRelationshipsInResult) {
698-
if (targetIdSelector.apply(possibleRelationship).equals(targetNodeId)) {
699-
700-
// Reduce the amount of relationships in the candidate list.
701-
// If this relationship got processed twice (OUTGOING, INCOMING), it is never needed again
702-
// and therefor should not be in the list.
703-
// Otherwise, for highly linked data it could potentially cause a StackOverflowError.
704-
String direction = relationshipDescription.getDirection().name();
705-
if (knownObjects.hasProcessedRelationshipCompletely("R" + direction + IdentitySupport.getElementId(possibleRelationship))) {
706-
relationshipsFromResult.remove(possibleRelationship);
707-
}
708-
// If the target is the same(equal) node, get the related object from the cache.
709-
// Avoiding the call to the map method also breaks an endless cycle of trying to finish
710-
// the property population of _this_ object.
711-
// The initial population will happen at the end of this mapping. This is sufficient because
712-
// it only affects properties not changing the instance of the object.
713-
Object mappedObject;
714-
if (fetchMore) {
715-
mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
716-
? knownObjects.getObject("N" + sourceNodeId)
717-
: map(possibleValueNode, concreteTargetNodeDescription, baseDescription, null, null, relationshipsFromResult, nodesFromResult);
718-
} else {
719-
Object objectFromStore = knownObjects.getObject("N" + targetNodeId);
720-
mappedObject = objectFromStore != null
721-
? objectFromStore
722-
: map(possibleValueNode, concreteTargetNodeDescription, baseDescription, null, null, relationshipsFromResult, nodesFromResult);
723-
}
695+
Neo4jPersistentEntity<?> concreteTargetNodeDescription =
696+
getMostConcreteTargetNodeDescription(genericTargetNodeDescription, possibleValueNode);
697+
698+
Set<Relationship> relationshipsProcessed = new HashSet<>();
699+
for (Relationship possibleRelationship : allMatchingTypeRelationshipsInResult) {
700+
if (targetIdSelector.apply(possibleRelationship).equals(targetNodeId)) {
724701

725-
if (relationshipDescription.hasRelationshipProperties()) {
726-
Object relationshipProperties;
727-
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
702+
// Reduce the amount of relationships in the candidate list.
703+
// If this relationship got processed twice (OUTGOING, INCOMING), it is never needed again
704+
// and therefor should not be in the list.
705+
// Otherwise, for highly linked data it could potentially cause a StackOverflowError.
706+
String direction = relationshipDescription.getDirection().name();
707+
if (knownObjects.hasProcessedRelationshipCompletely("R" + direction + IdentitySupport.getElementId(possibleRelationship))) {
708+
relationshipsFromResult.remove(possibleRelationship);
709+
}
710+
// If the target is the same(equal) node, get the related object from the cache.
711+
// Avoiding the call to the map method also breaks an endless cycle of trying to finish
712+
// the property population of _this_ object.
713+
// The initial population will happen at the end of this mapping. This is sufficient because
714+
// it only affects properties not changing the instance of the object.
715+
Object mappedObject;
728716
if (fetchMore) {
729-
relationshipProperties = map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
717+
mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
718+
? knownObjects.getObject("N" + sourceNodeId)
719+
: map(possibleValueNode, concreteTargetNodeDescription, baseDescription, null, null, relationshipsFromResult, nodesFromResult);
720+
} else {
721+
Object objectFromStore = knownObjects.getObject("N" + targetNodeId);
722+
mappedObject = objectFromStore != null
723+
? objectFromStore
724+
: map(possibleValueNode, concreteTargetNodeDescription, baseDescription, null, null, relationshipsFromResult, nodesFromResult);
725+
}
726+
727+
if (relationshipDescription.hasRelationshipProperties()) {
728+
Object relationshipProperties;
729+
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
730+
if (fetchMore) {
731+
relationshipProperties = map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
732+
} else {
733+
Object objectFromStore = knownObjects.getObject(IdentitySupport.getPrefixedElementId(possibleRelationship, relationshipDescription.getDirection().name()));
734+
relationshipProperties = objectFromStore != null
735+
? objectFromStore
736+
: map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
737+
}
738+
relationshipsAndProperties.add(relationshipProperties);
739+
mappedObjectHandler.accept(possibleRelationship.type(), relationshipProperties);
730740
} else {
731-
Object objectFromStore = knownObjects.getObject(IdentitySupport.getPrefixedElementId(possibleRelationship, relationshipDescription.getDirection().name()));
732-
relationshipProperties = objectFromStore != null
733-
? objectFromStore
734-
: map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
741+
mappedObjectHandler.accept(possibleRelationship.type(), mappedObject);
735742
}
736-
relationshipsAndProperties.add(relationshipProperties);
737-
mappedObjectHandler.accept(possibleRelationship.type(), relationshipProperties);
738-
} else {
739-
mappedObjectHandler.accept(possibleRelationship.type(), mappedObject);
743+
relationshipsProcessed.add(possibleRelationship);
740744
}
741-
relationshipsProcessed.add(possibleRelationship);
742745
}
746+
allMatchingTypeRelationshipsInResult.removeAll(relationshipsProcessed);
743747
}
744-
allMatchingTypeRelationshipsInResult.removeAll(relationshipsProcessed);
745748
}
746749
} else if (!relationshipListEmptyOrNull) {
747750
for (Value relatedEntity : list.asList(Function.identity())) {

0 commit comments

Comments
 (0)