On Fri, 28 Feb 2025 15:36:33 GMT, Mikhail Yankelevich <[email protected]> wrote:
>> Fixes deletion of non-existent (i.e., already-deleted) files in
>> `SocketChannelConnectionSetup`. Confirmed the fix using
>>
>>
>> make build-microbenchmark
>> build/linux-x64/jdk/bin/java \
>> -jar build/linux-x64/images/test/micro/benchmarks.jar \
>> -f 1 -wi 1 -i 1 -t 2 SocketChannelConnectionSetup
>>
>>
>> Note the `-t 2` in the JMH arguments – this was triggering the reported
>> `NoSuchFileException`.
>>
>> `make test TEST=micro:SocketChannelConnectionSetup` is passing as expected
>> too.
>
> test/micro/org/openjdk/bench/java/net/SocketChannelConnectionSetup.java line
> 79:
>
>> 77:
>> 78:
>> 79: private ServerSocketChannel getInetServerSocketChannel() throws
>> IOException {
>
> Minor: Do you think it will be easier to follow if this method was removed
> and line 72 directly called the `ServerSocketChannel.open().bind(null);`?
> Seems a bit of an overkill to have a separate method for a single line.
>
> But of course it it up to you, I don't have any strong opinion about this.
> Only worth considering if there's going to be another revision anyway.
Thanks for sparing time to review @myankelev, I appreciated it! 🙇
AFAIU, the author tried to encapsulate the details of `ServerSocketChannel`
instance creation in a method pair and I tend to agree with this split. I will
keep things as is.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23843#discussion_r1975889580
