<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 6/13/13 11:54 AM, Hiroshi Yamauchi
wrote:<br>
</div>
<blockquote
cite="mid:CAASM7NK7ooi+gCFzKTmeM4GNWU=p+6PCdBXp6etTjk2sjaDwyQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div class="h5">
> > - this patch is for DefNew/CMS only it
seems. Is this correct?<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div style="">Yes.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div class="h5">
> ><br>
> > - in the Hotspot code base explicit != NULL
checks are done for whatever<br>
> > reason. To keep the same style it would be
nice to update the checks<br>
> > whether to do the sampling ("if
(CMSEdenChunksRecordAlways &&<br>
> > _next_gen)") accordingly.<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div style="">I'll fix these.</div>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div class="h5">
> ><br>
> > (the next point is not an issue of the
patch)<br>
> > - the check whether there is a _next_gen is
odd - I do not think that<br>
> > DefNew works as a generation without an
older generation, but I'm not<br>
> > sure.<br>
> You're correct that DefNew needs to have a
_next_gen.<br>
><br>
> > Looking at other similar code checking for
_next_gen != NULL, the<br>
> > response is typically to update _next_gen
and then asserting that<br>
> > _next_gen is != NULL. E.g.<br>
> ><br>
> > if (_next_gen == NULL) {<br>
> > GenCollectedHeap* gch =
GenCollectedHeap::heap();<br>
> > _next_gen = gch->next_gen(this);<br>
> > assert(_next_gen != NULL,<br>
> > "This must be the youngest gen,
and not the only gen");<br>
> > }<br>
> ><br>
> > Jon?<br>
><br>
> Yes, except in a very few situations, _next_gen
should be set.<br>
> Exceptions I would expect is during
initialization. At the point<br>
> Thomas indicates and assertion would be
sufficient.<br>
><br>
> assert(_next_gen != NULL, "...");<br>
<br>
</div>
</div>
Thanks.</blockquote>
<div><br>
</div>
<div style="">I assume this discussion is out of scope of
this patch. Let me know otherwise.</div>
<div><br>
</div>
</div>
<br>
</div>
</div>
</blockquote>
Yes, not your code so fixing should not be part of your patch.<br>
</body>
</html>