[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Fwd: Re: CRR (S): 7099849: G1: include heap region information in hs_err files
From:       bengt.rutisson () oracle ! com (Bengt Rutisson)
Date:       2011-10-31 10:06:35
Message-ID: 4EAE732B.2090209 () oracle ! com
[Download RAW message or body]


Missed to include the runtime list in my earlier reply.

(Tony, I think you got the wrong hotspot-runtime address in your 
original email. Using hotspot-runtime-dev at openjdk.java.net instead.)

Bengt

-------- Original Message --------
Subject: 	Re: CRR (S): 7099849: G1: include heap region information in 
hs_err files
Date: 	Mon, 31 Oct 2011 10:55:58 +0100
From: 	Bengt Rutisson <bengt.rutisson at oracle.com>
To: 	hotspot-gc-dev at openjdk.java.net



Hi Tony,

Thanks for fixing this! I think it will be great to have this
information in the hs_err file.

It is also good that you clean up the print() and print_on() structure.
But I think you can take it one step further.

In collectedHeap.hpp you now have:

virtual void print() const {
     print_on(tty);
   }

But CollectedHeap inherits AllocatedObj (through CHeapObj). And
AllocatedObj defines print() as:

void AllocatedObj::print() const       { print_on(tty); }

So, I think you can just remove the print() method in CollectedHeap.
This will also get rid of the shadowing of AllocatedObj::print() by
CollectedHeap::print(). That looks kind of fishy to me.


Also, a nit pick, but the whitespace changes at lines 414 and 417 in
universe.hpp seem kind of strange to me.

Finally, just for the record, we have tests that run without setting
-Xms or -Xmx. If such a test ends up on a large machine we can end up
with many more than 1000-2000 regions. I have seen ten times as many
regions.

It might be unlikely that customers run without limiting the heap size
on this large machines, but we will run into hs_err files with 10s of
thousands of heap regions in our own testing. Personally I think the
information is useful enough for this to be acceptable. One alternative
would of course be to log this in a separate hs_err_g1 file...

Bengt


On 2011-10-12 18:49, Tony Printezis wrote:
> Hi all,
> 
> (I'm also copying the runtime alias, as this change will concern them
> too)
> 
> I'd like to get a couple of reviews for this change:
> 
> http://cr.openjdk.java.net/~tonyp/7099849/webrev.0/
> 
> Some background: when trying to track down issues in G1 it is often
> very helpful to know what type of regions the heap has and/or get
> information on a particular region (whether it's humongous, whether
> it's young, how full it is, etc.). We thought it'd be a good idea to
> include the per-region information in the hs_err file to always have
> it available after a crash.
> 
> I don't think the changes in the webrev are too controversial. The
> reason I wanted to give a heads up to both groups was to point out
> that this change will increase the size of the hs_err files when G1 is
> used. It's common to have 1,000 to 2,000 regions in the heap, if
> larger heaps are used (much fewer if smaller heaps are used), which
> means that the hs_err file will have this many extra lines. Does
> anyone have any concerns about this?
> 
> I attached an example hs_err file obtained from a workspace with this
> change applied.
> 
> BTW, I also cleaned up a bit the way the print() methods on the heap
> are defined (I pushed the default behavior to the superclass, where
> possible).
> 
> Tony
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20111031/a1e154b9/attachment.html \



[prev in list] [next in list] [prev in thread] [next in thread]