RFR(XS): 8064471: Port 8013895: G1: G1SummarizeRSetStats output on Linux needs improvement to AIX

[David Holmes]

Hi Gunter,

On 11/11/2014 11:23 PM, Haug, Gunter wrote:
> Hi All,
>
> The change '8013895:  (G1: G1SummarizeRSetStats output on Linux needs improvement)' makes use of getrusage() to retrieve accurate per-thread data on resource usage. We can use exactly the same code on AIX to achieve this.
>
> Please review the following change:
>
> http://cr.openjdk.java.net/~goetz/webrevs/8064471-aixTime/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8064471

I have a couple of comments on this code which presumably also apply to 
the orginal :(

First this comment is no longer applicable (actually it was never 
applicable to AIX!):

   // For now, we say that linux does not support vtime. I have no idea
   // whether it can actually be made to (DLD, 9/13/05).

Second this calculation seems wrong:

return (double) (usage.ru_utime.tv_sec + usage.ru_stime.tv_sec) + 
(double) (usage.ru_utime.tv_usec + usage.ru_stime.tv_usec) / (1000 * 1000);

To me this performs integer division (ie truncation_) then converts the 
resulting integer to a double. I would expect to see additional 
parentheses (even if not needed, for clarity):

return (double) (usage.ru_utime.tv_sec + usage.ru_stime.tv_sec) + 
((double) (usage.ru_utime.tv_usec + usage.ru_stime.tv_usec)) / (1000 * 
1000);

or more simply divide by a floating-point value:

return (double) (usage.ru_utime.tv_sec + usage.ru_stime.tv_sec) + 
(usage.ru_utime.tv_usec + usage.ru_stime.tv_usec) / (1000.0 * 1000);

and you don't need two double casts regardless as the expression will be 
of type double as soon as there is one operand of type double. So that 
should reduce to:

return usage.ru_utime.tv_sec + usage.ru_stime.tv_sec + 
(usage.ru_utime.tv_usec + usage.ru_stime.tv_usec) / (1000.0 * 1000);

Cheers,
David

> Thanks,
> Gunter
>