Hi Roger,
A few comments on the updated version.
284 long cpuMillis =
Long.valueOf(args[nextArg++]);
285 long cpuTarget = getCpuTime() + cpuMillis
* 1_000_000L;
286 while (getCpuTime() < cpuTarget) {
287 // burn the cpu until the time is up
Are there interger overflow issues in adding to the result of getCpuTime()?
Should the time values be a function of the timeout factor the test is
running under?
If the answer to both of these is "no," then I think this is okay as-is.
Thanks,
-Joe
On 7/8/2015 1:07 PM, Roger Riggs wrote:
Hi Joe,
Updated the webrev in place.
http://cr.openjdk.java.net/~rriggs/webrev-info-8098852/
On 7/7/2015 7:59 PM, joe darcy wrote:
Hi Roger,
Generally looks okay; a few comments and suggestions
114 long cpulooptime = 100; // 100 ms
How about
cpuLoopTime
ok
instead? Same comment for the other variables that don't follow
camel-case conventions.
I fixed a few others too.
284 long cpumillis =
Long.valueOf(args[nextArg++]);
285 long cpuTarget = getCpuTime() +
cpumillis * 1_000_000L;
286 while (getCpuTime() < cpuTarget) {
Is it correct to multiply cpu-millis by 1e6 rather than 1e3?
Yes, CpuTime is in nanos = millis * 1000 (micros) * 1000
Thanks, Roger
-Joe
On 7/7/2015 11:52 AM, Roger Riggs wrote:
Please review this ProcessHandle test change to cleanup intermittent
failures.
The cpuloop timing uses the cputime of the spawned process and the
test runs fewer iterations and relaxes the threshold.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-info-8098852/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8098852
Thanks, Roger
