On Wed, 14 Jun 2023 11:02:04 GMT, Daniel Fuchs <[email protected]> wrote:
>> The HttpClient uses `Instant.now()` to create deadlines for timeouts. This
>> could have undesirable effects since `Instant.now()` is linked to the wall
>> clock, which is not monotonic. This fix changes the HttpClient to use a
>> monotonic instant source based on `System.nanoTime()` for the purpose of
>> setting and comparing deadlines.
>
> 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()`?
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.
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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229428984
PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229432322
PR Review Comment: https://git.openjdk.org/jdk/pull/14450#discussion_r1229430561
