Use same time source for waiting and measuring #7905
Conversation
The test now uses the same time source to measure simulated working times the build operation executor uses to measure the durations of build operations. Thus, this commit should fix the flakiness on Windows were `System.currentTimeMillis()` is not as reliable as on other operating systems.
marcphilipp
added
from:member
a:chore
| def elapsed | ||
| while ((elapsed = org.gradle.internal.time.Time.currentTimeMillis() - start) < $durationMillis) { | ||
| Thread.sleep($durationMillis - elapsed) | ||
| } |
There was a problem hiding this comment.
This doesn't seem very right.
Suppose start = 100, then after Thread.sleep(100), when elapsed = org.gradle.internal.time.Time.currentTimeMillis() - start is calculated, current time might be 200, 300, 1000, or more, i.e. If the thread happens to wait for a long time between Thread.sleep($durationMillis) and (elapsed = org.gradle.internal.time.Time.currentTimeMillis() - start), elapsed might be very large, then $durationMillis - elapsed is negative. Thread.sleep says
* @throws IllegalArgumentException
* if the value of {@code millis} is negative
Can't we just replace original code to
long start = org.gradle.internal.time.Time.currentTimeMillis()
while (org.gradle.internal.time.Time.currentTimeMillis() - start < $sleepMillis) {
Thread.sleep($sleepMillis)
}
if System.currentTimeMillis() implementation is not accurate on Windows?
There was a problem hiding this comment.
durationMillis - elapsed is never negative because the while loop is only entered/repeated if elapsed < durationMillis.
There was a problem hiding this comment.
Ah... sorry I was wrong. Then no questions.
| while (System.currentTimeMillis() - start < $sleepDurationMillis) { | ||
| Thread.sleep(${sleepDurationMillis / 2}) | ||
| } | ||
| ${simulateWork(sleepDurationMillis)} |
There was a problem hiding this comment.
Also, shouldn't this be sleepDurationMillis / 2?
There was a problem hiding this comment.
Please ignore the original / 2. That was a spurious attempt to avoid waiting twice as long as intended.
marcphilipp
deleted the
marc/fixes/ProjectConfigurationProgressEventCrossVersionSpec
branch
|
We'll see if it actually fixes it. 😉 |
The test now uses the same time source to measure simulated working
times the build operation executor uses to measure the durations of
build operations. Thus, this commit should fix the flakiness on Windows
were
System.currentTimeMillis()is not as reliable as on otheroperating systems.