From 6222f053a3b5c036a8b49ed1dee42d476570d97d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sat, 4 Jan 2025 22:27:23 +0100 Subject: [PATCH 01/18] Add speed test for cache creation --- .../transit/speed_test/SpeedTest.java | 17 +++++++++++++++++ .../speed_test/model/timer/SpeedTestTimer.java | 3 +++ 2 files changed, 20 insertions(+) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index ca4e85eed84..3228a2f8fa8 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -13,11 +13,13 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; +import java.util.stream.IntStream; import org.opentripplanner.TestServerContext; import org.opentripplanner.datastore.OtpDataStore; import org.opentripplanner.framework.application.OtpAppException; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.raptor.configure.RaptorConfig; +import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.response.RoutingResponse; import org.opentripplanner.routing.framework.DebugTimingAggregator; import org.opentripplanner.routing.graph.Graph; @@ -192,6 +194,9 @@ public void runTest() { } updateTimersWithGlobalCounters(); + + timeTransferCacheComputation(); + printProfileStatistics(); saveTestCasesToResultFile(); System.err.println("\nSpeedTest done! " + projectInfo().getVersionString()); @@ -335,6 +340,18 @@ private void saveTestCasesToResultFile() { } } + private void timeTransferCacheComputation() { + IntStream + .of(1, 2, 3, 4, 5, 6, 7) + .forEach(reluctance -> { + RouteRequest routeRequest = new RouteRequest(); + routeRequest.withPreferences(b -> b.withWalk(c -> c.withReluctance(reluctance))); + timer.transferCacheTimer.record(() -> + timetableRepository.getTransitLayer().initTransferCacheForRequest(routeRequest) + ); + }); + } + /** * Add "static" transit statistics and JVM memory usages to the "timers" logging. */ diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java index 48a0548d28e..59a83c9ea2c 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java @@ -39,6 +39,9 @@ public class SpeedTestTimer { List.of(loggerRegistry) ); private final MeterRegistry uploadRegistry = MeterRegistrySetup.getRegistry().orElse(null); + + public final Timer transferCacheTimer = registry.timer("transfer_cache_computation"); + private boolean groupResultByTestCaseCategory = false; public static int nanosToMillisecond(long nanos) { From d3e67b050b6eb1faf57e86e0686fcd977c295709 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sat, 4 Jan 2025 22:28:04 +0100 Subject: [PATCH 02/18] Run speed test on branch --- .github/workflows/performance-test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml index 5e55e158a6f..e15f13fc796 100644 --- a/.github/workflows/performance-test.yml +++ b/.github/workflows/performance-test.yml @@ -4,6 +4,7 @@ on: push: branches: - dev-2.x + - transfer-cache-speed-test jobs: perf-test: From e3044e63364f69f1208864e77204c00db2078c93 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 09:24:47 +0100 Subject: [PATCH 03/18] Improve recording --- .../opentripplanner/transit/speed_test/SpeedTest.java | 5 +++-- .../transit/speed_test/model/timer/SpeedTestTimer.java | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index 3228a2f8fa8..2415b940dde 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -346,8 +346,9 @@ private void timeTransferCacheComputation() { .forEach(reluctance -> { RouteRequest routeRequest = new RouteRequest(); routeRequest.withPreferences(b -> b.withWalk(c -> c.withReluctance(reluctance))); - timer.transferCacheTimer.record(() -> - timetableRepository.getTransitLayer().initTransferCacheForRequest(routeRequest) + timer.recordTimer( + "transfer_cache_computation", + () -> timetableRepository.getTransitLayer().initTransferCacheForRequest(routeRequest) ); }); } diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java index 59a83c9ea2c..d5ff56b9d53 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java @@ -139,6 +139,15 @@ public void globalCount(String meterName, long count) { } } + public void recordTimer(String meterName, Runnable runnable) { + if (uploadRegistry != null) { + registry.add(uploadRegistry); + var timer = registry.timer(meterName); + timer.record(runnable); + registry.remove(uploadRegistry); + } + } + /** * Calculate the total time mean for the given timer. If the timer is not * found {@link #NOT_AVAILABLE} is returned. This can be the case in unit tests, From e429a2d868d4153b030ea94ec078725ed3665013 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 09:33:37 +0100 Subject: [PATCH 04/18] Reverse order of measurements --- .../opentripplanner/transit/speed_test/SpeedTest.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index 2415b940dde..f986322bfb4 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -193,9 +193,11 @@ public void runTest() { } } + measureTransferCacheComputation(); + updateTimersWithGlobalCounters(); - timeTransferCacheComputation(); + timer.finishUp(); printProfileStatistics(); saveTestCasesToResultFile(); @@ -340,7 +342,11 @@ private void saveTestCasesToResultFile() { } } - private void timeTransferCacheComputation() { + /** + * Measure how long it takes to compute the transfer cache. + */ + private void measureTransferCacheComputation() { + System.out.println("Measuring transfer cache computation"); IntStream .of(1, 2, 3, 4, 5, 6, 7) .forEach(reluctance -> { @@ -370,7 +376,6 @@ private void updateTimersWithGlobalCounters() { timer.globalCount("jvm_max_memory", runtime.maxMemory()); timer.globalCount("jvm_total_memory", runtime.totalMemory()); timer.globalCount("jvm_used_memory", runtime.totalMemory() - runtime.freeMemory()); - timer.finishUp(); } /** From e1167a92867ad101368f3412dbc626133ec53601 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 10:31:46 +0100 Subject: [PATCH 05/18] Remove log --- .../java/org/opentripplanner/transit/speed_test/SpeedTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index f986322bfb4..9b6b5b1ed68 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -346,7 +346,6 @@ private void saveTestCasesToResultFile() { * Measure how long it takes to compute the transfer cache. */ private void measureTransferCacheComputation() { - System.out.println("Measuring transfer cache computation"); IntStream .of(1, 2, 3, 4, 5, 6, 7) .forEach(reluctance -> { From 9232f2de6d28024b5bf07a797bdb84c3d1f2100d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 10:33:16 +0100 Subject: [PATCH 06/18] Remove unused field --- .../transit/speed_test/model/timer/SpeedTestTimer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java index d5ff56b9d53..3f9313fae69 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java @@ -40,8 +40,6 @@ public class SpeedTestTimer { ); private final MeterRegistry uploadRegistry = MeterRegistrySetup.getRegistry().orElse(null); - public final Timer transferCacheTimer = registry.timer("transfer_cache_computation"); - private boolean groupResultByTestCaseCategory = false; public static int nanosToMillisecond(long nanos) { From 6f02877db551c0cbbc1f1ec400fa27c77d4bca46 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 22:29:29 +0100 Subject: [PATCH 07/18] Small clean up in timer code --- .../transit/speed_test/model/timer/SpeedTestTimer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java index 3f9313fae69..847cbad2a14 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java @@ -185,7 +185,7 @@ public String name(String name, Meter.Type type, String unit) { } private String capitalize(String name) { - if (name.length() != 0 && !Character.isUpperCase(name.charAt(0))) { + if (!name.isEmpty() && !Character.isUpperCase(name.charAt(0))) { char[] chars = name.toCharArray(); chars[0] = Character.toUpperCase(chars[0]); return new String(chars); @@ -218,8 +218,8 @@ public static Result merge(Collection results) { for (Result it : results) { any = it; - min = it.min < min ? it.min : min; - max = it.max > max ? it.max : max; + min = Math.min(it.min, min); + max = Math.max(it.max, max); totTime += it.totTime; count += it.count; } From cc4f438cbf232008c5cc0bb2f23c8bd7806b6157 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 5 Jan 2025 22:40:04 +0100 Subject: [PATCH 08/18] Remove branch name --- .github/workflows/performance-test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml index e15f13fc796..5e55e158a6f 100644 --- a/.github/workflows/performance-test.yml +++ b/.github/workflows/performance-test.yml @@ -4,7 +4,6 @@ on: push: branches: - dev-2.x - - transfer-cache-speed-test jobs: perf-test: From 041d1bbedc9d7741e8b754762c8052da05ba3549 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 6 Jan 2025 11:03:01 +0100 Subject: [PATCH 09/18] Add Javadoc --- .../transit/speed_test/model/timer/SpeedTestTimer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java index 847cbad2a14..80970eaad0a 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/model/timer/SpeedTestTimer.java @@ -137,6 +137,9 @@ public void globalCount(String meterName, long count) { } } + /** + * Execute the runnable and record its runtime in the meter name passed in. + */ public void recordTimer(String meterName, Runnable runnable) { if (uploadRegistry != null) { registry.add(uploadRegistry); From 25ffb7342bb7990a5eea096062ca02157a419778 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 12:55:00 +0100 Subject: [PATCH 10/18] Extract separate test for transfer cache --- .../transit/speed_test/LoadModel.java | 7 ++ .../transit/speed_test/SetupHelper.java | 38 ++++++++++ .../transit/speed_test/SpeedTest.java | 25 ------- .../transit/speed_test/TransferCacheTest.java | 71 +++++++++++++++++++ 4 files changed, 116 insertions(+), 25 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/transit/speed_test/LoadModel.java create mode 100644 application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java create mode 100644 application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/LoadModel.java b/application/src/test/java/org/opentripplanner/transit/speed_test/LoadModel.java new file mode 100644 index 00000000000..5e5823c842b --- /dev/null +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/LoadModel.java @@ -0,0 +1,7 @@ +package org.opentripplanner.transit.speed_test; + +import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.standalone.config.BuildConfig; +import org.opentripplanner.transit.service.TimetableRepository; + +record LoadModel(Graph graph, TimetableRepository timetableRepository, BuildConfig buildConfig) {} diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java new file mode 100644 index 00000000000..b94cecc8a3e --- /dev/null +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java @@ -0,0 +1,38 @@ +package org.opentripplanner.transit.speed_test; + +import java.io.File; +import java.net.URI; +import org.opentripplanner.datastore.OtpDataStore; +import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.routing.graph.SerializedGraphObject; +import org.opentripplanner.standalone.config.ConfigModel; +import org.opentripplanner.standalone.config.OtpConfigLoader; +import org.opentripplanner.transit.service.TimetableRepository; +import org.opentripplanner.transit.speed_test.options.SpeedTestCmdLineOpts; + +/** + * A package-private helper class for setting up speed tests. + */ +class SetupHelper { + + static LoadModel loadGraph(File baseDir, URI path) { + File file = path == null + ? OtpDataStore.graphFile(baseDir) + : path.isAbsolute() ? new File(path) : new File(baseDir, path.getPath()); + SerializedGraphObject serializedGraphObject = SerializedGraphObject.load(file); + Graph graph = serializedGraphObject.graph; + + if (graph == null) { + throw new IllegalStateException(); + } + + TimetableRepository timetableRepository = serializedGraphObject.timetableRepository; + timetableRepository.index(); + graph.index(timetableRepository.getSiteRepository()); + return new LoadModel(graph, timetableRepository, serializedGraphObject.buildConfig); + } + + static void loadOtpFeatures(SpeedTestCmdLineOpts opts) { + ConfigModel.initializeOtpFeatures(new OtpConfigLoader(opts.rootDir()).loadOtpConfig()); + } +} diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index 9b6b5b1ed68..11400c07be0 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -13,13 +13,11 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; -import java.util.stream.IntStream; import org.opentripplanner.TestServerContext; import org.opentripplanner.datastore.OtpDataStore; import org.opentripplanner.framework.application.OtpAppException; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.raptor.configure.RaptorConfig; -import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.response.RoutingResponse; import org.opentripplanner.routing.framework.DebugTimingAggregator; import org.opentripplanner.routing.graph.Graph; @@ -29,7 +27,6 @@ import org.opentripplanner.service.vehiclerental.internal.DefaultVehicleRentalService; import org.opentripplanner.standalone.OtpStartupInfo; import org.opentripplanner.standalone.api.OtpServerRequestContext; -import org.opentripplanner.standalone.config.BuildConfig; import org.opentripplanner.standalone.config.ConfigModel; import org.opentripplanner.standalone.config.DebugUiConfig; import org.opentripplanner.standalone.config.OtpConfigLoader; @@ -193,8 +190,6 @@ public void runTest() { } } - measureTransferCacheComputation(); - updateTimersWithGlobalCounters(); timer.finishUp(); @@ -342,22 +337,6 @@ private void saveTestCasesToResultFile() { } } - /** - * Measure how long it takes to compute the transfer cache. - */ - private void measureTransferCacheComputation() { - IntStream - .of(1, 2, 3, 4, 5, 6, 7) - .forEach(reluctance -> { - RouteRequest routeRequest = new RouteRequest(); - routeRequest.withPreferences(b -> b.withWalk(c -> c.withReluctance(reluctance))); - timer.recordTimer( - "transfer_cache_computation", - () -> timetableRepository.getTransitLayer().initTransferCacheForRequest(routeRequest) - ); - }); - } - /** * Add "static" transit statistics and JVM memory usages to the "timers" logging. */ @@ -390,8 +369,4 @@ private List trimItineraries(RoutingResponse routingResponse) { } return stream.limit(opts.numOfItineraries()).toList(); } - - /* inline classes */ - - record LoadModel(Graph graph, TimetableRepository timetableRepository, BuildConfig buildConfig) {} } diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java new file mode 100644 index 00000000000..499d16691d4 --- /dev/null +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java @@ -0,0 +1,71 @@ +package org.opentripplanner.transit.speed_test; + +import static org.opentripplanner.standalone.configure.ConstructApplication.creatTransitLayerForRaptor; +import static org.opentripplanner.transit.speed_test.support.AssertSpeedTestSetup.assertTestDateHasData; + +import java.util.stream.IntStream; +import org.opentripplanner.framework.application.OtpAppException; +import org.opentripplanner.routing.api.request.RouteRequest; +import org.opentripplanner.standalone.OtpStartupInfo; +import org.opentripplanner.transit.service.TimetableRepository; +import org.opentripplanner.transit.speed_test.model.timer.SpeedTestTimer; +import org.opentripplanner.transit.speed_test.options.SpeedTestCmdLineOpts; +import org.opentripplanner.transit.speed_test.options.SpeedTestConfig; + +/** + * Test how long it takes to compute the transfer cache. + */ +public class TransferCacheTest { + + public static void main(String[] args) { + try { + OtpStartupInfo.logInfo("Run transfer cache test"); + // Given the following setup + SpeedTestCmdLineOpts opts = new SpeedTestCmdLineOpts(args); + var config = SpeedTestConfig.config(opts.rootDir()); + SetupHelper.loadOtpFeatures(opts); + var model = SetupHelper.loadGraph(opts.rootDir(), config.graph); + var timetableRepository = model.timetableRepository(); + var buildConfig = model.buildConfig(); + + var timer = new SpeedTestTimer(); + + assertTestDateHasData(timetableRepository, config, buildConfig); + + // Creating transitLayerForRaptor should be integrated into the TimetableRepository, but for now + // we do it manually here + creatTransitLayerForRaptor(timetableRepository, config.transitRoutingParams); + + measureTransferCacheComputation(timer, timetableRepository); + + timer.setUp(false); + timer.finishUp(); + } catch (OtpAppException ae) { + System.err.println(ae.getMessage()); + System.exit(1); + } catch (Exception e) { + System.err.println(e.getMessage()); + e.printStackTrace(System.err); + System.exit(1); + } + } + + /** + * Measure how long it takes to compute the transfer cache. + */ + private static void measureTransferCacheComputation( + SpeedTestTimer timer, + TimetableRepository timetableRepository + ) { + IntStream + .range(1, 7) + .forEach(reluctance -> { + RouteRequest routeRequest = new RouteRequest(); + routeRequest.withPreferences(b -> b.withWalk(c -> c.withReluctance(reluctance))); + timer.recordTimer( + "transfer_cache_computation", + () -> timetableRepository.getTransitLayer().initTransferCacheForRequest(routeRequest) + ); + }); + } +} From 8dde5b0423e5e4c1751e55b01981007f5ca2d521 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 13:12:28 +0100 Subject: [PATCH 11/18] Update CI config --- .github/workflows/performance-test.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml index 5e55e158a6f..d4376c242c5 100644 --- a/.github/workflows/performance-test.yml +++ b/.github/workflows/performance-test.yml @@ -91,7 +91,7 @@ jobs: cp otp-shaded/target/otp-shaded-*-SNAPSHOT.jar otp.jar java -Xmx32G -jar otp.jar --build --save test/performance/${{ matrix.location }}/ - - name: Run speed test + - name: Run RAPTOR speed test if: matrix.profile == 'core' || github.ref == 'refs/heads/dev-2.x' env: PERFORMANCE_INFLUX_DB_PASSWORD: ${{ secrets.PERFORMANCE_INFLUX_DB_PASSWORD }} @@ -113,3 +113,12 @@ jobs: with: name: ${{ matrix.location }}-flight-recorder path: application/${{ matrix.location }}-speed-test.jfr + + - name: Run transfer cache speed test + if: matrix.profile == 'core' || github.ref == 'refs/heads/dev-2.x' + env: + PERFORMANCE_INFLUX_DB_PASSWORD: ${{ secrets.PERFORMANCE_INFLUX_DB_PASSWORD }} + SPEEDTEST_LOCATION: ${{ matrix.location }} + MAVEN_OPTS: "-Xmx50g -Dmaven.repo.local=/home/lenni/.m2/repository/" + run: | + mvn --projects application exec:java -Dexec.mainClass="org.opentripplanner.transit.speed_test.TransferCacheTest" -Dexec.classpathScope=test -Dexec.args="--dir=test/performance/${{ matrix.location }}" From 3a8faa0fc7702f7c591bbb2e7446c0f446d95533 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 13:19:00 +0100 Subject: [PATCH 12/18] Run test on branch --- .github/workflows/performance-test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml index d4376c242c5..ddff431872a 100644 --- a/.github/workflows/performance-test.yml +++ b/.github/workflows/performance-test.yml @@ -4,6 +4,7 @@ on: push: branches: - dev-2.x + - transfer-cache-speed-test jobs: perf-test: From 1bcdf1178ff6399adc2e271de47f5f1f57d58db5 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 13:55:43 +0100 Subject: [PATCH 13/18] Create transit layer first --- .../transit/speed_test/TransferCacheTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java index 499d16691d4..84b45c0e73a 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java @@ -28,14 +28,14 @@ public static void main(String[] args) { var timetableRepository = model.timetableRepository(); var buildConfig = model.buildConfig(); - var timer = new SpeedTestTimer(); - - assertTestDateHasData(timetableRepository, config, buildConfig); - // Creating transitLayerForRaptor should be integrated into the TimetableRepository, but for now // we do it manually here creatTransitLayerForRaptor(timetableRepository, config.transitRoutingParams); + assertTestDateHasData(timetableRepository, config, buildConfig); + + var timer = new SpeedTestTimer(); + measureTransferCacheComputation(timer, timetableRepository); timer.setUp(false); From 96a0fccd2d6d4b9531412e38f0befcd44f05b73c Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 15:11:27 +0100 Subject: [PATCH 14/18] Initialize before adding any data --- .../transit/speed_test/TransferCacheTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java index 84b45c0e73a..1c41cf67044 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java @@ -28,17 +28,17 @@ public static void main(String[] args) { var timetableRepository = model.timetableRepository(); var buildConfig = model.buildConfig(); + var timer = new SpeedTestTimer(); + timer.setUp(false); + // Creating transitLayerForRaptor should be integrated into the TimetableRepository, but for now // we do it manually here creatTransitLayerForRaptor(timetableRepository, config.transitRoutingParams); assertTestDateHasData(timetableRepository, config, buildConfig); - var timer = new SpeedTestTimer(); - measureTransferCacheComputation(timer, timetableRepository); - timer.setUp(false); timer.finishUp(); } catch (OtpAppException ae) { System.err.println(ae.getMessage()); From 5c9774975c368d3b78987fa80c0bbe91cc8a30e5 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 16:54:46 +0100 Subject: [PATCH 15/18] Re-use setup logic --- .../transit/speed_test/SpeedTest.java | 25 ++----------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index 11400c07be0..2a3add223e8 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -152,8 +152,8 @@ public static void main(String[] args) { // Given the following setup SpeedTestCmdLineOpts opts = new SpeedTestCmdLineOpts(args); var config = SpeedTestConfig.config(opts.rootDir()); - loadOtpFeatures(opts); - var model = loadGraph(opts.rootDir(), config.graph); + SetupHelper.loadOtpFeatures(opts); + var model = SetupHelper.loadGraph(opts.rootDir(), config.graph); var timetableRepository = model.timetableRepository(); var buildConfig = model.buildConfig(); var graph = model.graph(); @@ -269,27 +269,6 @@ private RoutingResponse performRouting(TestCase testCase) { /* setup helper methods */ - private static void loadOtpFeatures(SpeedTestCmdLineOpts opts) { - ConfigModel.initializeOtpFeatures(new OtpConfigLoader(opts.rootDir()).loadOtpConfig()); - } - - private static LoadModel loadGraph(File baseDir, URI path) { - File file = path == null - ? OtpDataStore.graphFile(baseDir) - : path.isAbsolute() ? new File(path) : new File(baseDir, path.getPath()); - SerializedGraphObject serializedGraphObject = SerializedGraphObject.load(file); - Graph graph = serializedGraphObject.graph; - - if (graph == null) { - throw new IllegalStateException(); - } - - TimetableRepository timetableRepository = serializedGraphObject.timetableRepository; - timetableRepository.index(); - graph.index(timetableRepository.getSiteRepository()); - return new LoadModel(graph, timetableRepository, serializedGraphObject.buildConfig); - } - private void initProfileStatistics() { for (SpeedTestProfile key : opts.profiles()) { workerResults.put(key, new ArrayList<>()); From 179592cce4e0c41568ea18e84db931f571ba4703 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 14 Jan 2025 17:34:27 +0100 Subject: [PATCH 16/18] Don't run on branch anymore --- .github/workflows/performance-test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml index ddff431872a..d4376c242c5 100644 --- a/.github/workflows/performance-test.yml +++ b/.github/workflows/performance-test.yml @@ -4,7 +4,6 @@ on: push: branches: - dev-2.x - - transfer-cache-speed-test jobs: perf-test: From cda87ff049fe240ffb509948dd108fb461b70bc6 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 15 Jan 2025 10:12:16 +0100 Subject: [PATCH 17/18] Incorporate review feedback --- .../opentripplanner/transit/speed_test/SetupHelper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java index b94cecc8a3e..86383d6d94e 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SetupHelper.java @@ -2,6 +2,7 @@ import java.io.File; import java.net.URI; +import javax.annotation.Nullable; import org.opentripplanner.datastore.OtpDataStore; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.SerializedGraphObject; @@ -15,7 +16,7 @@ */ class SetupHelper { - static LoadModel loadGraph(File baseDir, URI path) { + static LoadModel loadGraph(File baseDir, @Nullable URI path) { File file = path == null ? OtpDataStore.graphFile(baseDir) : path.isAbsolute() ? new File(path) : new File(baseDir, path.getPath()); @@ -23,7 +24,9 @@ static LoadModel loadGraph(File baseDir, URI path) { Graph graph = serializedGraphObject.graph; if (graph == null) { - throw new IllegalStateException(); + throw new IllegalStateException( + "Could not find graph at %s".formatted(file.getAbsolutePath()) + ); } TimetableRepository timetableRepository = serializedGraphObject.timetableRepository; From 4d50bdc0d61b746c26cd85f1af00b067654861c4 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Thu, 16 Jan 2025 15:25:45 +0100 Subject: [PATCH 18/18] Combine catch blocks --- .../opentripplanner/transit/speed_test/TransferCacheTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java index 1c41cf67044..e6c8de67688 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/TransferCacheTest.java @@ -40,9 +40,6 @@ public static void main(String[] args) { measureTransferCacheComputation(timer, timetableRepository); timer.finishUp(); - } catch (OtpAppException ae) { - System.err.println(ae.getMessage()); - System.exit(1); } catch (Exception e) { System.err.println(e.getMessage()); e.printStackTrace(System.err);