Kotlin / kotlinx-benchmark

Kotlin multiplatform benchmarking toolkit
Apache License 2.0
512 stars 41 forks source link

Update JMH version #147

Open fzhinkin opened 1 year ago

fzhinkin commented 1 year ago

When running benchmarks on JVM, an old version of JMH is used by default (https://github.com/Kotlin/kotlinx-benchmark/blob/master/gradle.properties#L5). In addition to numerous improvements made in JMH, there were also some bugs fixed. For example, v1.21 can't properly parse the output of perfasm profiler on aarch64 (see https://bugs.openjdk.org/browse/JDK-8252498).

Everyone can specify their own favorite version of JMH manually, but it would be nice to have a more recent version by default.

fzhinkin commented 8 months ago

Writing the jmhVersion every single time is pissing me off, so I attempted to bump up the version. Everything works fine as long as there are no tasks specifying older version using jmhVersion property. There are no problems if there's only one such a jvm benchmarking target.

However, if there are two or more jvm benchmarking targets and (as it's not prohibited) different jmh versions are set, like here: https://github.com/Kotlin/kotlinx-benchmark/blob/231ce03739278a5f0f9805dc2760d568dd7fe64e/integration/src/test/kotlin/kotlinx/benchmark/integration/SourceSetAsBenchmarkTargetTest.kt#L21 If one such target depend on another, jmh-dependencies will be updated to the latest version specified (i.e. if there is target with a default jmh version, let's say we updated it to 1.37, and also there's a target with explicitly specified jmh 1.21, then for the latter runtime dependency on jmh will be updated to 1.37; it wouldn't be that bad, but an old JMH version may still be used to generate sources and these sources will be incompatible with a newer JMH version used at compile time).

One solution would be to use strict versions, but that fails short if targets somehow depend on each other (in the example above, where targets are based on source sets, one compilation task will inherit some default dependencies from another and when strict versions are used, that will lead to a conflict).

I'm not sure if the original intent was to allow using different JMH version for different targets within the same project. Assuming it was not, it might be worth reconsidering how this option works and either throwing an exception when there are multiple tasks with different JMH versions, or deprecating target-level option altogether and configuring JMH-version for the whole project only (there's already a benchmarks_jmh_version property for that). Or, of course, we may pursue correct behavior when different tasks use different JMH versions, but personally, I don't see a lot of value there.

@qurbonzoda WDYT?

qurbonzoda commented 7 months ago

I also can’t see any benefits of testing with different JMH versions, and I doubt it was intended to allow the use of different JMH versions. As a first step, we could prohibit explicitly specifying different jmhVersion in different JVM benchmark targets. For targets without an explicit jmhVersion, we could default to the version specified in another target or to the default version if none is specified.

Later, we should design an API to specify the JMH version for all JVM targets.

qurbonzoda commented 7 months ago

Everyone can specify their own favourite version of JMH manually, but it would be nice to have a more recent version by default.

I support the idea. Given that the possible negative effects on already written benchmarks are researched, I would be happy to see the default version updated to a newer version.