Port newer benchmarks to JMH and run those benchmarks using GitHub Actions

[Liversticks]

Issue #, if available:

#44

Description of changes:

Port the newer benchmarks (the ones using citylots2.json to Java Microbenchmark Harness (JMH). These benchmarks measure throughput in operations/second. The project has been restructured so the JMH benchmarks live in the benchmark module and the main library code lives in the ruler module.

Existing benchmarks are kept for correctness check (they have JUnit asserts).

Add a new step in CI that runs the benchmarks after running the library tests. We can change the number of iterations (warmup/regular) which affects how much time the benchmarks take to run.

Benchmark / Performance (for source code changes):

By default, JMH runs 5 warmup iterations and 5 regular iterations. This current benchmark suite takes about 2 hours and 20 minutes to run.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[baldawar]

Hey Oliver,
thanks for getting out so quick. I will take a deeper look tomorrow evening but had two questions upfront

1/ is the pkg restructure necessary? If so, would it be better to it in a separate PR to avoid coupling with you change (or is that a lot of hassle)
2/ does the benchmark needs its own pom file if we add JMH as a dependency in the main ruler pkg ? I'm worried that with two POM files we might eventually diverge the dependency chain.

[baldawar]

didn't quite understand why this is necessary.

[Liversticks]

It's not, I added it to trigger a commit for CI. I'll remove it.

[baldawar]

I guess this class is no longer necessary

[Liversticks]

Makes sense, the methods under benchmark are still tested in the other tests so I will remove the Benchmarks class.

[baldawar]

test themselves LGTM but the folder restrucutre still bothers me. the other 2 hr runtime is concerining as well. Do you know if that's happening because of we build against multiple version of java (7, 11, and 17).

[Liversticks]

test themselves LGTM but the folder restrucutre still bothers me. the other 2 hr runtime is concerining as well. Do you know if that's happening because of we build against multiple version of java (7, 11, and 17).

By default, JMH runs 5 warmup iterations and 5 iterations for each benchmark and runs each benchmark in 5 forks. Each iteration (warmup and regular) takes 10 seconds. There are 17 benchmarks. Not including overhead time (example: reading in citylots2 file), the default time is hence 17 * (5 * (10 *(5 + 5))) = 8500 s or 2.36 hours. The CI builds against multiple versions of Java in parallel so that is not the root cause of the long runtime.

To reduce runtime, we can lower the parameters. If we run 1 warmup iteration, 3 regular iterations, 3 forks, and set the iteration runtime to 5 s, the new runtime is 17 * (3 * (5 * (3 + 1))) = 1020 s or 17 minutes.

[baldawar]

To reduce runtime, we can lower the parameters. If we run 1 warmup iteration, 3 regular iterations, 3 forks, and set the iteration runtime to 5 s, the new runtime is 17 * (3 * (5 * (3 + 1))) = 1020 s or 17 minutes.

17 mins is fine IMO. Ideally, we'd want to run the shorter runs of every PR and then periodically run the longer tests (like once a week) but that can be done later.

[Liversticks]

Hey Oliver, thanks for getting out so quick. I will take a deeper look tomorrow evening but had two questions upfront

1/ is the pkg restructure necessary? If so, would it be better to it in a separate PR to avoid coupling with you change (or is that a lot of hassle) 2/ does the benchmark needs its own pom file if we add JMH as a dependency in the main ruler pkg ? I'm worried that with two POM files we might eventually diverge the dependency chain.

I'd like to avoid the package restructure but it's the recommended way to add JMH in: https://github.com/openjdk/jmh. From the README there:

NOTE: JMH is not intended to be used in the same way as a typical testing library such as JUnit. Simply adding the jmh-core jar file to your build is not enough to be able to run benchmarks.

[baldawar]

hey @Liversticks marking this as closed for now. we can reopen when you or me have bandwidth to look at this again.