diff --git a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimings.java b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimings.java new file mode 100644 index 0000000000..a55e8ef135 --- /dev/null +++ b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimings.java @@ -0,0 +1,35 @@ +/* + * 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 . + */ +package org.neo4j.gds.applications.algorithms.pathfinding; + +/** + * This is finalised timings for one algorithm processing. Going for a good old struct here + */ +public class AlgorithmProcessingTimings { + public final long preProcessingMillis; + public final long computeMillis; + public final long postProcessingMillis; + + AlgorithmProcessingTimings(long preProcessingMillis, long computeMillis, long postProcessingMillis) { + this.preProcessingMillis = preProcessingMillis; + this.computeMillis = computeMillis; + this.postProcessingMillis = postProcessingMillis; + } +} diff --git a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimingsBuilder.java b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimingsBuilder.java new file mode 100644 index 0000000000..0da9410f32 --- /dev/null +++ b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/AlgorithmProcessingTimingsBuilder.java @@ -0,0 +1,53 @@ +/* + * 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 . + */ +package org.neo4j.gds.applications.algorithms.pathfinding; + +/** + * This guy gathers timings, builder-stylee. + */ +class AlgorithmProcessingTimingsBuilder { + // This is a marker + private static final int NOT_AVAILABLE = -1; + + // timings + protected long preProcessingMillis = NOT_AVAILABLE; + protected long computeMillis = NOT_AVAILABLE; + protected long postProcessingMillis = NOT_AVAILABLE; // mutate or write timing + + public void withPreProcessingMillis(long preProcessingMillis) { + this.preProcessingMillis = preProcessingMillis; + } + + public void withComputeMillis(long computeMillis) { + this.computeMillis = computeMillis; + } + + public void withPostProcessingMillis(long postProcessingMillis) { + this.postProcessingMillis = postProcessingMillis; + } + + AlgorithmProcessingTimings build() { + return new AlgorithmProcessingTimings( + preProcessingMillis, + computeMillis, + postProcessingMillis + ); + } +} diff --git a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplate.java b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplate.java index 032aefae61..9a36cf9bd6 100644 --- a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplate.java +++ b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplate.java @@ -73,7 +73,10 @@ public > mutateOrWriteStep, ResultBuilder resultBuilder ) { + var timingsBuilder = new AlgorithmProcessingTimingsBuilder(); + Pair graphWithGraphStore = graphLoadAndValidationWithTiming( + timingsBuilder, graphName, configuration, resultBuilder @@ -82,18 +85,18 @@ public */ Pair graphLoadAndValidationWithTiming( + AlgorithmProcessingTimingsBuilder timingsBuilder, GraphName graphName, CONFIGURATION configuration, ResultBuilder resultBuilder ) { - try (ProgressTimer ignored = ProgressTimer.start(resultBuilder::withPreProcessingMillis)) { + try (ProgressTimer ignored = ProgressTimer.start(timingsBuilder::withPreProcessingMillis)) { // tee up the graph we want to work on var graphWithGraphStore = graphStoreCatalogService.getGraphWithGraphStore( graphName, @@ -131,12 +135,13 @@ RESULT_FROM_ALGORITHM computeWithTiming( + AlgorithmProcessingTimingsBuilder timingsBuilder, String humanReadableAlgorithmName, AlgorithmComputation algorithmComputation, ResultBuilder resultBuilder, Graph graph ) { - try (ProgressTimer ignored = ProgressTimer.start(resultBuilder::withComputeMillis)) { + try (ProgressTimer ignored = ProgressTimer.start(timingsBuilder::withComputeMillis)) { return computeWithMetric(humanReadableAlgorithmName, algorithmComputation, graph); } } @@ -161,13 +166,14 @@ private RESULT_FROM_ALGORITHM computeWithMetric( void mutateOrWriteWithTiming( Optional> mutateOrWriteStep, - ResultBuilder resultBuilder, + AlgorithmProcessingTimingsBuilder timingsBuilder, Graph graph, GraphStore graphStore, - RESULT_FROM_ALGORITHM result + RESULT_FROM_ALGORITHM result, + ResultBuilder resultBuilder ) { mutateOrWriteStep.ifPresent(step -> { - try (ProgressTimer ignored = ProgressTimer.start(resultBuilder::withPostProcessingMillis)) { + try (ProgressTimer ignored = ProgressTimer.start(timingsBuilder::withPostProcessingMillis)) { step.execute(graph, graphStore, result, resultBuilder); } }); diff --git a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ResultBuilder.java b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ResultBuilder.java index 3728b93c7a..ca5247f0cc 100644 --- a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ResultBuilder.java +++ b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ResultBuilder.java @@ -25,7 +25,7 @@ import java.util.Optional; /** - * This builder gathers data as part of algorithm processing. Timings and such. + * This builder gathers data as part of algorithm processing * You specialise it for use cases, but a lot of what it needs is generic, * and it is part of algorithm processing instrumentation. * In-layer generic usage includes injecting the Graph, hence it is a parameter to the build method. @@ -35,27 +35,13 @@ * where any given usage probably won't need all of them. */ public abstract class ResultBuilder { - // timings - protected long preProcessingMillis; - protected long computeMillis; - protected long postProcessingMillis = -1; // mutate or write timing + // a marker + private static final int NOT_AVAILABLE = -1; // union type: zero or more of these get populated by your own hooks - protected long nodeCount; - protected long nodePropertiesWritten; - protected long relationshipsWritten; - - public void withPreProcessingMillis(long preProcessingMillis) { - this.preProcessingMillis = preProcessingMillis; - } - - public void withComputeMillis(long computeMillis) { - this.computeMillis = computeMillis; - } - - public void withPostProcessingMillis(long postProcessingMillis) { - this.postProcessingMillis = postProcessingMillis; - } + protected long nodeCount = NOT_AVAILABLE; + protected long nodePropertiesWritten = NOT_AVAILABLE; + protected long relationshipsWritten = NOT_AVAILABLE; public void withNodeCount(long nodeCount) { this.nodeCount = nodeCount; @@ -78,6 +64,7 @@ public void withRelationshipsWritten(long relationshipsWritten) { public abstract RESULT_TO_CALLER build( Graph graph, GraphStore graphStore, - Optional result + Optional result, + AlgorithmProcessingTimings timings ); } diff --git a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ShortestPathWriteStep.java b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ShortestPathWriteStep.java index 524cb9c1a3..b2fc5d3f13 100644 --- a/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ShortestPathWriteStep.java +++ b/applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/ShortestPathWriteStep.java @@ -25,7 +25,6 @@ import org.neo4j.gds.api.IdMap; import org.neo4j.gds.api.nodeproperties.ValueType; import org.neo4j.gds.config.WriteRelationshipConfig; -import org.neo4j.gds.core.utils.ProgressTimer; import org.neo4j.gds.core.utils.progress.TaskRegistryFactory; import org.neo4j.gds.core.utils.progress.tasks.TaskProgressTracker; import org.neo4j.gds.core.write.ImmutableExportedRelationship; @@ -124,22 +123,20 @@ public void execute( var writeRelationshipType = configuration.writeRelationshipType(); /* - * The actual export, with timing. + * The actual export. * Notice that originally we had a CloseableResourceRegistry thing going on here - no longer. * Because all we are doing is, processing a stream using the exporter, synchronously. * We are not handing it out to upper layers for sporadic consumption. * It is done right here, and when we complete, the stream is exhausted. * We still explicitly close the stream tho because, yeah, confusion I guess. */ - try (ProgressTimer ignored = ProgressTimer.start(resultBuilder::withPostProcessingMillis)) { - var keys = createKeys(writeNodeIds, writeCosts); - var types = createTypes(writeNodeIds, writeCosts); + var keys = createKeys(writeNodeIds, writeCosts); + var types = createTypes(writeNodeIds, writeCosts); - var relationshipsWritten = relationshipStreamExporter.write(writeRelationshipType, keys, types); + var relationshipsWritten = relationshipStreamExporter.write(writeRelationshipType, keys, types); - // the final result is the side effect of writing to the database, plus this metadata - resultBuilder.withRelationshipsWritten(relationshipsWritten); - } + // the final result is the side effect of writing to the database, plus this metadata + resultBuilder.withRelationshipsWritten(relationshipsWritten); } } diff --git a/applications/algorithms/path-finding/src/test/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplateTest.java b/applications/algorithms/path-finding/src/test/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplateTest.java index 02ade098f4..98d833c9eb 100644 --- a/applications/algorithms/path-finding/src/test/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplateTest.java +++ b/applications/algorithms/path-finding/src/test/java/org/neo4j/gds/applications/algorithms/pathfinding/DefaultAlgorithmProcessingTemplateTest.java @@ -83,10 +83,11 @@ void shouldProcessStreamAlgorithm() { public Stream build( Graph graph, GraphStore graphStore, - Optional pathFindingResult + Optional pathFindingResult, + AlgorithmProcessingTimings timings ) { // we skip timings when no side effects requested - assertThat(postProcessingMillis).isEqualTo(-1); + assertThat(timings.postProcessingMillis).isEqualTo(-1); return Stream.of( "Huey", @@ -150,7 +151,8 @@ void shouldProcessMutateOrWriteAlgorithm() { public Long build( Graph actualGraph, GraphStore actualGraphStore, - Optional actualResult + Optional actualResult, + AlgorithmProcessingTimings timings ) { assertThat(actualGraph).isEqualTo(graph); assertThat(actualGraphStore).isEqualTo(graphStore); @@ -204,34 +206,37 @@ void shouldDoTimings() { ) { @Override Pair graphLoadAndValidationWithTiming( + AlgorithmProcessingTimingsBuilder timingsBuilder, GraphName graphName, CONFIGURATION configuration, ResultBuilder resultBuilder ) { - resultBuilder.withPreProcessingMillis(23); + timingsBuilder.withPreProcessingMillis(23); return Pair.of(mock(Graph.class), null); } @Override RESULT_FROM_ALGORITHM computeWithTiming( + AlgorithmProcessingTimingsBuilder timingsBuilder, String humanReadableAlgorithmName, AlgorithmComputation algorithmComputation, ResultBuilder resultBuilder, Graph graph ) { - resultBuilder.withComputeMillis(117); + timingsBuilder.withComputeMillis(117); return null; } @Override void mutateOrWriteWithTiming( Optional> mutateOrWriteStep, - ResultBuilder resultBuilder, + AlgorithmProcessingTimingsBuilder timingsBuilder, Graph graph, GraphStore graphStore, - RESULT_FROM_ALGORITHM resultFromAlgorithm + RESULT_FROM_ALGORITHM resultFromAlgorithm, + ResultBuilder resultBuilder ) { - resultBuilder.withPostProcessingMillis(87); + timingsBuilder.withPostProcessingMillis(87); } }; @@ -240,12 +245,13 @@ void mutateOrWriteWithTiming( public Map build( Graph graph, GraphStore graphStore, - Optional unused + Optional unused, + AlgorithmProcessingTimings timings ) { return Map.of( - "preProcessingMillis", preProcessingMillis, - "computeMillis", computeMillis, - "postProcessingMillis", postProcessingMillis + "preProcessingMillis", timings.preProcessingMillis, + "computeMillis", timings.computeMillis, + "postProcessingMillis", timings.postProcessingMillis ); } }; diff --git a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForMutateMode.java b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForMutateMode.java index f66bff8357..09920f2f09 100644 --- a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForMutateMode.java +++ b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForMutateMode.java @@ -21,6 +21,7 @@ import org.neo4j.gds.api.Graph; import org.neo4j.gds.api.GraphStore; +import org.neo4j.gds.applications.algorithms.pathfinding.AlgorithmProcessingTimings; import org.neo4j.gds.applications.algorithms.pathfinding.ResultBuilder; import org.neo4j.gds.config.ToMapConvertible; import org.neo4j.gds.paths.dijkstra.PathFindingResult; @@ -38,13 +39,14 @@ class PathFindingResultBuilderForMutateMode extends ResultBuilder pathFindingResult + Optional pathFindingResult, + AlgorithmProcessingTimings timings ) { return new PathFindingMutateResult( - preProcessingMillis, - computeMillis, + timings.preProcessingMillis, + timings.computeMillis, 0, // yeah, I don't understand it either :shrug: - postProcessingMillis, + timings.postProcessingMillis, relationshipsWritten, configuration.toMap() ); diff --git a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForStreamMode.java b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForStreamMode.java index 4cb63980ec..7838ba0cec 100644 --- a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForStreamMode.java +++ b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForStreamMode.java @@ -22,6 +22,7 @@ import org.neo4j.gds.api.Graph; import org.neo4j.gds.api.GraphStore; import org.neo4j.gds.api.NodeLookup; +import org.neo4j.gds.applications.algorithms.pathfinding.AlgorithmProcessingTimings; import org.neo4j.gds.applications.algorithms.pathfinding.ResultBuilder; import org.neo4j.gds.paths.dijkstra.PathFindingResult; import org.neo4j.graphdb.Path; @@ -50,7 +51,8 @@ public class PathFindingResultBuilderForStreamMode extends ResultBuilder build( Graph graph, GraphStore graphStore, - Optional result + Optional result, + AlgorithmProcessingTimings timings ) { if (result.isEmpty()) return Stream.of(); diff --git a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForWriteMode.java b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForWriteMode.java index 06c20319f5..c78ca1fa0b 100644 --- a/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForWriteMode.java +++ b/procedures/facade/src/main/java/org/neo4j/gds/procedures/pathfinding/PathFindingResultBuilderForWriteMode.java @@ -21,6 +21,7 @@ import org.neo4j.gds.api.Graph; import org.neo4j.gds.api.GraphStore; +import org.neo4j.gds.applications.algorithms.pathfinding.AlgorithmProcessingTimings; import org.neo4j.gds.applications.algorithms.pathfinding.ResultBuilder; import org.neo4j.gds.config.ToMapConvertible; import org.neo4j.gds.paths.dijkstra.PathFindingResult; @@ -39,13 +40,14 @@ class PathFindingResultBuilderForWriteMode extends ResultBuilder pathFindingResult + Optional pathFindingResult, + AlgorithmProcessingTimings timings ) { return new StandardWriteRelationshipsResult( - preProcessingMillis, - computeMillis, + timings.preProcessingMillis, + timings.computeMillis, 0, // yeah, I don't understand it either :shrug: - postProcessingMillis, + timings.postProcessingMillis, relationshipsWritten, configuration.toMap() );