Pls review 6173675/7003271
mandy.chung at oracle.com
Wed Jan 5 12:05:57 PST 2011
On 01/04/11 13:00, Paul Hohensee wrote:
> These two rfes implement per-thread approximate memory allocation
> 6173675 also adds multi-thread-id versions of getThreadCpuTime and
> *6173675 M&M: approximate memory allocation rate/amount per thread
> **7003271 Hotspot should track cumulative Java heap bytes allocated on
> a per-thread basis <http://monaco.sfbay.sun.com/detail.jsf?cr=7003271>
> *6173675 is the library part, while 7003271 is the Hotspot part.
> Webrevs here
I reviewed the jdk change.
Happy new year! Should the copyright year be 2011?
Can you include the unit tests in the webrev?
line 153-155: the spec allows the given ids array of zero-length. The
check should not be added. You may want to add a check in line 175 to
return infos if ids.length == 0.
Thanks for doing the refactoring. I think the check calling
isThreadCpuTimeEnabled() should be included in the
verifyCurrentThreadCpuTime and verifyThreadCpuTime methods. How about
having these 2 methods to return a boolean; i.e. return
boolean verifyThreadCpuTime(long ids)
The callers to the verifyThreadCpuTime(long) method will do its own
allocation of the resulting long. line 255-259 can be refactored as a
separate method (which can also be called by the getThreadAllocatedBytes
Similarly, the verifyThreadAllocatedMemory method can return a boolean
(isThreadAllocatedMemoryEnabled()) and have the caller to construct the
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the serviceability-dev