crac
crac copied to clipboard
8351402: [CRaC] Use System.nanoTime() in TimedWaitingTest
Replaces System.currentTimeMillis() with System.nanoTime() in TimedWaitingTest since the former can, in theory, jump back and forth and that may lead to the test failures.
Also adds a diagnostic assert.
Progress
- [x] Change must not contain extraneous whitespace
Issue
- JDK-8351402: [CRaC] Use System.nanoTime() in TimedWaitingTest (Bug - P3)
Reviewers
- Anton Kozlov (@AntonKozlov - Reviewer)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/crac.git pull/209/head:pull/209
$ git checkout pull/209
Update a local copy of the PR:
$ git checkout pull/209
$ git pull https://git.openjdk.org/crac.git pull/209/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 209
View PR using the GUI difftool:
$ git pr show -t 209
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/crac/pull/209.diff
Using Webrev
:wave: Welcome back tpushkin! A progress list of the required criteria for merging this PR into crac will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@TimPushkin This change is no longer ready for integration - check the PR body for details.
/integrate
@TimPushkin Your change (at version 2f1a1e148208b567232cdaed4346b72fb662d3d3) is now ready to be sponsored by a Committer.
There have been private discussions about the test. Whether it should test:
- the specification compliance: that timed waiting is not shorter than it should be according to the spec — in this case using nanoTime should probably be the fix, or
- the implementation: that CRaC doesn't manipulate nanoTime in a way that makes timed waiting shorter than it would be without CRaC (this should be a superset of the first point) — in this case we should use some other way to access monotonic time but not nanoTime.
I think we've leaned towards the second, so it should be checked that the non-monotonicity of systemTimeMillis is really the reason of the test failures. So I'll make this a draft for now.
I refreshed my memories about the tests, and the first approach looks good for me. There is a comment in the bug. Sorry for misleading you.
Closing in favor of #221