RFR: 8309939: HttpClient should not use Instant.now() as Instant source for deadlines [v2]

[Daniel Fuchs]

On Wed, 14 Jun 2023 11:14:02 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into TimeSource
>>  - 8309939
>
> src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java line 132:
> 
>> 130:     }
>> 131: 
>> 132:     ConnectionPool(long clientId, TimeSource timeSource) {
> 
> Hello Daniel, as far as I can see, no one calls this constructor outside of this very class and this seems to be always passed a constant `TimeSource.source()`. So maybe we don't have to have a `timeSource` field in this class and instead just use `TimeSource.now()` at call sites within this class (and `ExpiryList`) whenever we want `now()`?

This constructor should in fact take an InstantSource, not a TimeSource. It was added in case we wanted to upgrade the whitebox ConnectionPoolTest to create an instance of ConnectionPool with a custom InstantSource.

> src/java.net.http/share/classes/jdk/internal/net/http/common/TimeSource.java line 76:
> 
>> 74:                 // The use of Integer.MAX_VALUE is arbitrary.
>> 75:                 // Any value not too close to Long.MAX_VALUE
>> 76:                 // would do.
> 
> The `Integer.MAX_VALUE` gets used in `isInWindow()` method implementation, through the use of `TIME_WINDOW` member. Should this comment about `Integer.MAX_VALUE` instead be moved as a field comment for `TIME_WINDOW`? That way it's closer to where the `Integer.MAX_VALUE` is actually used.

Done

> src/java.net.http/share/classes/jdk/internal/net/http/common/TimeSource.java line 82:
> 
>> 80:         }
>> 81: 
>> 82:         // @ForceInline
> 
> Should these couple of commented out `@ForceInline` be deleted?

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229539076
PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229540292
PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229540099