improve eksperimental Dijkstra by dropping unnecessary result store, layering responsibilities better, and generally simplifying

Author: lassewesth

applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/PathFindingAlgorithmsMutateModeBusinessFacade.java

@@ -21,8 +21,6 @@
 
 import org.neo4j.gds.RelationshipType;
 import org.neo4j.gds.api.GraphName;
-import org.neo4j.gds.api.ResultStore;
-import org.neo4j.gds.api.nodeproperties.ValueType;
 import org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel;
 import org.neo4j.gds.applications.algorithms.machinery.AlgorithmProcessingTemplate;
 import org.neo4j.gds.applications.algorithms.machinery.AlgorithmProcessingTemplateConvenience;
@@ -53,8 +51,9 @@
 import org.neo4j.gds.steiner.SteinerTreeResult;
 import org.neo4j.gds.traversal.RandomWalkMutateConfig;
 
-import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.AStar;
 import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.BFS;
@@ -243,9 +242,10 @@ public <RESULT> RESULT singlePairShortestPathDijkstra(
     public <RESULT> RESULT singlePairShortestPathDijkstraWithPaths(
         GraphName graphName,
         ShortestPathDijkstraMutateConfig configuration,
-        ResultStore resultStore,
+        Map<String, Stream<PathUsingInternalNodeIds>> pathStore,
         ResultBuilder<ShortestPathDijkstraMutateConfig, PathFindingResult, RESULT, Void> resultBuilder
     ) {
+        var sideEffect = new StorePathsSideEffect(pathStore, configuration.mutateRelationshipType());
 
         return algorithmProcessingTemplate.processAlgorithmAndAnySideEffects(
             Optional.empty(),
@@ -257,10 +257,9 @@ public <RESULT> RESULT singlePairShortestPathDijkstraWithPaths(
             DimensionTransformer.DISABLED,
             () -> estimationFacade.singlePairShortestPathDijkstra(configuration),
             (graph, __) -> pathFindingAlgorithms.singlePairShortestPathDijkstra(graph, configuration),
-            Optional.of(new StorePathsSideEffect(resultStore, configuration.mutateRelationshipType(), List.of("totalCost", "nodeIds", "costs"), List.of(ValueType.DOUBLE, ValueType.LONG_ARRAY, ValueType.DOUBLE_ARRAY))),
+            Optional.of(sideEffect),
             new MutateResultRenderer<>(configuration, resultBuilder)
         );
-
     }
 
     public <RESULT> RESULT singlePairShortestPathYens(

applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/PathUsingInternalNodeIds.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.gds.applications.algorithms.pathfinding;
+
+/**
+ * Back in the day, this would be a DTO I guess
+ */
+public record PathUsingInternalNodeIds(long sourceNode, long targetNode, long[] nodeIds, double[] costs, double totalCost) {}

applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/StorePathsSideEffect.java

@@ -19,70 +19,54 @@
  */
 package org.neo4j.gds.applications.algorithms.pathfinding;
 
-import org.neo4j.gds.api.ExportedRelationship;
-import org.neo4j.gds.api.ResultStore;
-import org.neo4j.gds.api.ResultStoreEntry;
-import org.neo4j.gds.api.nodeproperties.ValueType;
 import org.neo4j.gds.applications.algorithms.machinery.SideEffect;
 import org.neo4j.gds.core.loading.GraphResources;
-import org.neo4j.gds.core.utils.progress.JobId;
-import org.neo4j.gds.paths.PathResult;
 import org.neo4j.gds.paths.dijkstra.PathFindingResult;
-import org.neo4j.values.storable.Value;
-import org.neo4j.values.storable.Values;
 
-import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Stream;
 
-public class StorePathsSideEffect implements SideEffect<PathFindingResult, Void> {
-    private final ResultStore resultStore;
+/**
+ * This is our hook for storing Dijkstra paths in a place where they can later be found.
+ * I.e. we stick them in a map, keyed by relationship type as a pseudo parameter.
+ * The paths themselves are represented as internal node ids, costs of the hops between them, and the sum of the costs.
+ * It is the responsibility of outer layers to map the internal node ids back into user space.
+ */
+class StorePathsSideEffect implements SideEffect<PathFindingResult, Void> {
+    private final Map<String, Stream<PathUsingInternalNodeIds>> paths;
     private final String relationshipTypeAsString;
-    private final List<String> propertyKeys;
-    private final List<ValueType> propertyTypes;
 
-    public StorePathsSideEffect(
-        ResultStore resultStore,
-        String relationshipTypeAsString,
-        List<String> propertyKeys,
-        List<ValueType> propertyTypes
+    StorePathsSideEffect(
+        Map<String, Stream<PathUsingInternalNodeIds>> paths,
+        String relationshipTypeAsString
     ) {
-        this.resultStore = resultStore;
+        this.paths = paths;
         this.relationshipTypeAsString = relationshipTypeAsString;
-        this.propertyKeys = propertyKeys;
-        this.propertyTypes = propertyTypes;
     }
 
     @Override
     public Optional<Void> process(GraphResources graphResources, Optional<PathFindingResult> pathFindingResult) {
         if (pathFindingResult.isEmpty()) {
             return Optional.empty();
         }
-        var actualPathResult  = pathFindingResult.get();
-        Stream<ExportedRelationship> relationshipStream = actualPathResult.mapPaths(
-            pathResult -> new ExportedRelationship(
+
+        var actualPathResult = pathFindingResult.get();
+
+        // just record the paths, no conversions, that happens later
+        var pathStream = actualPathResult.mapPaths(
+            pathResult -> new PathUsingInternalNodeIds(
                 pathResult.sourceNode(),
                 pathResult.targetNode(),
-                createValues(pathResult)
-            ));
-
-        ResultStoreEntry.RelationshipStream streamEntry = new ResultStoreEntry.RelationshipStream(
-            relationshipTypeAsString,
-            propertyKeys,
-            propertyTypes,
-            relationshipStream,
-            graphResources.graph()::toOriginalNodeId
+                pathResult.nodeIds(),
+                pathResult.costs(),
+                pathResult.totalCost()
+            )
         );
-        resultStore.add(JobId.parse(this.relationshipTypeAsString), streamEntry);
 
-        return Optional.empty();
-    }
+        paths.put(relationshipTypeAsString, pathStream);
 
-    private Value[] createValues(PathResult pathResult) {
-        return new Value[]{
-            Values.doubleValue(pathResult.totalCost()),
-            Values.longArray(pathResult.nodeIds()),
-            Values.doubleArray(pathResult.costs())
-        };
+        // we have no interesting metadata
+        return Optional.empty();
     }
 }