Re: RFR: 8314759: VirtualThread.parkNanos timeout adjustment when pinned should be replaced
On Thu, 24 Aug 2023 11:55:20 GMT, Daniel Fuchs wrote: > Although... what happens if parkOnCarrierThread is called with a negative > value (as that could happen here)? Not an issue, nothing has changed there. - PR Comment: https://git.openjdk.org/jdk/pull/15405#issuecomment-1691569181
Re: RFR: 8314759: VirtualThread.parkNanos timeout adjustment when pinned should be replaced
On Wed, 23 Aug 2023 16:41:23 GMT, Alan Bateman wrote: > If yielding fails due to the pinning then VirtualThread.parkNanos parks on > the carrier thread with the remaining time. The calculation of the remaining > time needs to be replaced so that it obviously uses the difference between > the start and end time in the calculation. The current code isn't correct for > cases where System.nanoTimes return a negative value or when parking for > durations close to Long.MAX_VALUE (292 years). The change isn't really > testable so there aren't any test changes included. Although... what happens if parkOnCarrierThread is called with a negative value (as that could happen here)? - PR Comment: https://git.openjdk.org/jdk/pull/15405#issuecomment-1691537547
Re: RFR: 8314759: VirtualThread.parkNanos timeout adjustment when pinned should be replaced
On Wed, 23 Aug 2023 16:41:23 GMT, Alan Bateman wrote: > If yielding fails due to the pinning then VirtualThread.parkNanos parks on > the carrier thread with the remaining time. The calculation of the remaining > time needs to be replaced so that it obviously uses the difference between > the start and end time in the calculation. The current code isn't correct for > cases where System.nanoTimes return a negative value or when parking for > durations close to Long.MAX_VALUE (292 years). The change isn't really > testable so there aren't any test changes included. LGTM - Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15405#pullrequestreview-1593413969
Re: RFR: 8314759: VirtualThread.parkNanos timeout adjustment when pinned should be replaced
On Wed, 23 Aug 2023 16:41:23 GMT, Alan Bateman wrote: > If yielding fails due to the pinning then VirtualThread.parkNanos parks on > the carrier thread with the remaining time. The calculation of the remaining > time needs to be replaced so that it obviously uses the difference between > the start and end time in the calculation. The current code isn't correct for > cases where System.nanoTimes return a negative value or when parking for > durations close to Long.MAX_VALUE (292 years). The change isn't really > testable so there aren't any test changes included. Looks fine. - Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15405#pullrequestreview-1593141786
Re: RFR: 8314759: VirtualThread.parkNanos timeout adjustment when pinned should be replaced
On Wed, 23 Aug 2023 16:41:23 GMT, Alan Bateman wrote: > If yielding fails due to the pinning then VirtualThread.parkNanos parks on > the carrier thread with the remaining time. The calculation of the remaining > time needs to be replaced so that it obviously uses the difference between > the start and end time in the calculation. The current code isn't correct for > cases where System.nanoTimes return a negative value or when parking for > durations close to Long.MAX_VALUE (292 years). The change isn't really > testable so there aren't any test changes included. Marked as reviewed by aturbanov (Committer). - PR Review: https://git.openjdk.org/jdk/pull/15405#pullrequestreview-1592895422
