RFR: 8309939: HttpClient should not use Instant.now() as Instant source for deadlines [v2]
Jaikiran Pai
jpai at openjdk.org
Wed Jun 14 11:21:00 UTC 2023
On Wed, 14 Jun 2023 11:02:04 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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
More information about the net-dev
mailing list