<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Still waiting for a review, but have a
new version:<br>
<br>
Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-7097567">https://bugs.openjdk.java.net/browse/JDK-7097567</a>
<br>
Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/7097567/webrev.04/">http://cr.openjdk.java.net/~drwhite/7097567/webrev.04/</a><br>
<br>
Changes in rev 04 vs 02:<br>
- Updated for GC source restructuring.<br>
- Removed
<meta http-equiv="content-type" content="text/html; charset=utf-8">
"_in_young_gc_mode" field, which had been deleted by a bug fix
earlier but had been mistakenly resurrected.<br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
- Moved yc_type()
<meta http-equiv="content-type" content="text/html; charset=utf-8">
from G1CollectedHeap to G1CollectorState.<br>
<br>
Thanks!<br>
<br>
- Derek<br>
<br>
On 5/12/15 5:26 PM, Derek White wrote:<br>
</div>
<blockquote cite="mid:55527000.4080204@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Mikael,<br>
<br>
Thanks for the review. I just got back to this. Comments inline,
and new webrev below...<br>
<br>
On 1/28/15 5:03 AM, Mikael Gerdin wrote:<br>
</div>
<blockquote cite="mid:54C8B3F5.4090105@oracle.com" type="cite">Hi
Derek, <br>
<br>
On 2015-01-27 23:41, Derek White wrote: <br>
<blockquote type="cite">Review request! <br>
<br>
This is a change Ramki did before he left that didn't get
checked in. <br>
The basic idea is to move a bunch fields that encapsulate
collector <br>
state from G1CollectorPolicy into a separate class "
G1CollectorState". <br>
<br>
Thanks in advance! <br>
<br>
- Derek <br>
<br>
/Bug/: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-7097567">https://bugs.openjdk.java.net/browse/JDK-7097567</a>
<br>
/Webrev/: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/7097567/webrev.00/">http://cr.openjdk.java.net/~drwhite/7097567/webrev.00/</a>
<br>
</blockquote>
<br>
This is just a high level review to begin with, I didn't look
too carefully at all the state changes. <br>
<br>
I agree that it's a good idea to factor out the state changes
but I'm not convinced that the CollectorState should logically
be a member of the collector policy object. <br>
To me it feels more like a G1 heap would have a policy object
and a state object as members. <br>
</blockquote>
I talked this over with Thomas and now understand the issue. I
also sucked up some state fields from
<meta http-equiv="content-type" content="text/html; charset=utf-8">
g1CollectedHeap into g1CollectorState.
<blockquote cite="mid:54C8B3F5.4090105@oracle.com" type="cite"> If
the policy object needs to interact with the state object they
can of course do that via a pointer to the state object from the
policy. <br>
</blockquote>
OK<br>
<blockquote cite="mid:54C8B3F5.4090105@oracle.com" type="cite">
Besides that I don't like the fact that the state object is
declared in the g1CollectorPolicy.hpp header, I'd prefer it if
you could move that to a separate new header file. <br>
</blockquote>
OK<br>
<blockquote cite="mid:54C8B3F5.4090105@oracle.com" type="cite">
The change to compactibleFreeListSpace seems like an accident
since it has nothing to do with G1 state changes whatsoever. <br>
</blockquote>
OK<br>
<br>
- Derek<br>
<br>
---------- NEW WEBREV ----------<br>
<br>
Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-7097567">https://bugs.openjdk.java.net/browse/JDK-7097567</a>
<br>
Webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/7097567/webrev.02/">http://cr.openjdk.java.net/~drwhite/7097567/webrev.02/</a><br>
<br>
I also added follow-on enhancement request for turning
G1CollectorState into a real state machine.<br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<ul class="breadcrumbs">
<li><a moz-do-not-send="true" id="key-val" rel="4779486"
href="https://bugs.openjdk.java.net/browse/JDK-8080226">JDK-8080226</a>
G1: Replace collector state booleans with explicit state
variable(s) <br>
</li>
</ul>
<br>
<br>
</blockquote>
<br>
</body>
</html>