RFR: 7903451: JMH: Refactor internal Optional usages [v2]

Andrei Rybak duke at openjdk.org
Sun Apr 2 08:44:33 UTC 2023


On Wed, 29 Mar 2023 11:20:54 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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())
>         ));

Fixed in e7547c8.

> jmh-core/src/main/java/org/openjdk/jmh/util/Optional.java line 55:
> 
>> 53:     }
>> 54: 
>> 55:     public T orElseGet(Supplier<T> alternativeSupplier) {
> 
> `orElseFrom`?

I used the [name of the same method from `j.u.Optional`]( https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#orElseGet-java.util.function.Supplier-), because switching to `j.u.Optional` was the whole idea behind the original state of the PR.

> 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;

Fixed in 5dadaff.

-------------

PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1155267802
PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1155267794
PR Review Comment: https://git.openjdk.org/jmh/pull/98#discussion_r1155267568


More information about the jmh-dev mailing list