RFR: 7903451: JMH: Refactor internal Optional usages [v2]
Aleksey Shipilev
shade at openjdk.org
Wed Mar 29 11:24:15 UTC 2023
On Tue, 28 Mar 2023 12:54:58 GMT, Andrei Rybak <duke at openjdk.org> wrote:
>> This is a follow-up to the email https://mail.openjdk.org/pipermail/jmh-dev/2023-January/003562.html
>>
>> `java.util.Optional` isn't `Serializeable`. Because of that `org.openjdk.jmh.runner.options.TestOptions` fails with `java.io.NotSerializableException: java.util.Optional`, when `org.openjdk.jmh.util.Optional` is completely replaced with `java.util.Optional`. See [test results on branch `java-util-optional`](https://github.com/rybak/jmh/actions/runs/4326921418), ([one commit](https://github.com/rybak/jmh/commit/25b9cb382be7285a51446976247d036ac855fc91) on top of initial state of this PR).
>>
>> I didn't look further into possibility of removing `Optional`s as fields of class `CommandLineOptions`. I think that the refactoring up until the drop-in replacement to `java.util.Optional` might still be worth it, at least from the point of making it more approachable for people who are familiar with `java.util.Optional`. So here are these changes for your consideration. Details are in the commit messages.
>
> Andrei Rybak has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains two new commits since the last revision:
>
> - Runner: replace orAnother() with orElseGet()
>
> Refactor usage of method org.openjdk.jmh.util.Optional#orAnother in
> class Runner using new method orElseGet() with lazy `Supplier`s as per
> comment added in commit db1bc365 (7902576: Lazy query for current JVM
> args in Runner, 2020-01-20). Clean up imports, while we're here.
> - OptionsBuilder: don't pass empty Optional to orAnother()
>
> Passing an empty Optional to method orAnother() of class
> org.openjdk.jmh.util.Optional is a no-op. Stop doing it in getters
> getJvmArgs(), getJvmArgsAppend(), and getJvmArgsPrepend() of class
> OptionsBuilder.
jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java line 481:
> 479: () -> benchmark.getJvmArgs().orElseGet(
> 480: () -> ManagementFactory.getRuntimeMXBean().getInputArguments())));
> 481:
Indentation looks a bit odd to me...
Suggestion:
jvmArgs.addAll(options.getJvmArgs()
.orElseGet(() -> benchmark.getJvmArgs()
.orElseGet(() -> ManagementFactory.getRuntimeMXBean().getInputArguments())
));
jmh-core/src/main/java/org/openjdk/jmh/util/Optional.java line 55:
> 53: }
> 54:
> 55: public T orElseGet(Supplier<T> alternativeSupplier) {
`orElseFrom`?
jmh-core/src/main/java/org/openjdk/jmh/util/Optional.java line 56:
> 54:
> 55: public T orElseGet(Supplier<T> alternativeSupplier) {
> 56: return val != null ? val : alternativeSupplier.get();
Matching style for surrounding code:
Suggestion:
return (val == null) ? alternativeSupplier.get() : val;
-------------
PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1151780674
PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1150690264
PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1150689052
More information about the jmh-dev
mailing list