<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
I made changes for AbortableHeapClosure.<br>
<br>
- classes derived from HeapClosure in which doHeapRegion always
returns false<br>
are changed from bool doHeapRegion to void doHeapRegion.<br>
<br>
- classes with a doHeapRegion which sometimes returns true now
derive from<br>
AbortableHeapClosure and have bool doHeapRegionAbortable.<br>
<br>
- At first I had AbortableHeapClosure be a subclass of HeapClosure.<br>
But I think it's better that AbortableHeapClosure is not a
subclass of HeapClosure<br>
so methods which take a HeapClosure won't also accept an
AbortableHeapClosure.<br>
I think this means duplication some methods such as <br>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> void G1CollectedHeap::heap_region_iterate(HeapRegionClosure* cl) const {
_hrm.iterate(cl);
}
<span class="new" style="color: blue; font-weight: bold;">+ void G1CollectedHeap::heap_region_iterate(AbortableHeapRegionClosure* cl) const {</span>
<span class="new" style="color: blue; font-weight: bold;">+ _hrm.iterate(cl);</span>
<span class="new" style="color: blue; font-weight: bold;">+ }
</span>
<span class="new" style="color: blue; font-weight: bold;"><span class="changed" style="color: blue;">void HeapRegionManager::iterate(RegionClosure* blk)</span>
</span><span class="changed" style="color: blue;">void HeapRegionManager::iterate(AbortableHeapRegionClosure* blk)
There's probably a better c++ way to this but I don't know it.
Here's a webrev if you want to take a quick look:
<a class="moz-txt-link-freetext" href="http://bussund0416.us.oracle.com/export/users/jprovino/hs-rt-closure/hotspot/webrev">http://bussund0416.us.oracle.com/export/users/jprovino/hs-rt-closure/hotspot/webrev</a>
joe
</span></pre>
<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://bussund0416.us.oracle.com/export/users/jprovino/hs-rt-closure/hotspot/webrev/">http://bussund0416.us.oracle.com/export/users/jprovino/hs-rt-closure/hotspot/webrev/</a><br>
<div class="moz-cite-prefix">On 6/8/2015 10:44 AM, Stefan Karlsson
wrote:<br>
</div>
<blockquote cite="mid:5575AA42.2080206@oracle.com" type="cite">On
2015-06-08 16:37, Thomas Schatzl wrote:
<br>
<blockquote type="cite">Hi Joe,
<br>
<br>
On Thu, 2015-06-04 at 15:12 -0400, Joseph Provino wrote:
<br>
<blockquote type="cite">I just ran across some code in
<br>
g1CollectedHeap::collection_set_iterate_from()
<br>
that looks a little strange. It's been there since the first
revision 342.
<br>
<br>
if (cl->doHeapRegion(cur) && false) {
<br>
cl->incomplete();
<br>
return;
<br>
}
<br>
<br>
Is there any reason why this isn't just
<br>
<br>
cl->doHeapRegion(cur);
<br>
</blockquote>
file a bug and enjoy an easy cleanup for your path to a
Committer...
<br>
<br>
The HeapRegionClosure has a way of sending an "abort"
notification due
<br>
to error to the caller, which is that "cl->incomplete()"
call.
<br>
However in many if not all cases it is not used because either
the work
<br>
that needs to be done cannot fail, or any failure is really
fatal
<br>
anyway.
<br>
<br>
That's probably why the strange code - but you'll probably find
more of
<br>
that the deeper you dig.
<br>
<br>
I would replace that code by the call to cl->doHeapRegion()
and
<br>
guarantee() that its return value is always true in this case.
<br>
</blockquote>
<br>
You should probably change
G1CollectedHeap::collection_set_iteratate as well, since none of
the callers pass a closure that returns true from doHeapRegion.
<br>
<br>
Erik H suggested that we should create new closure named
AbortableHeapRegionClosure and get rid of the abort mechanism from
HeapRegionClosure.
<br>
<br>
StefanK
<br>
<br>
<blockquote type="cite">
<br>
Thanks,
<br>
Thomas
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>