<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Thomas,<br>
<br>
</div>
<blockquote type="cite"
cite="mid:1500024904.3458.8.camel@oracle.com">
<pre wrap="">Hi all,
can I have reviews for this change that tries to clean up (and only
clean up) the G1CMBitMap class (and the surrounding helper classes) a
bit?
What has been done:
- fix naming
- improve visibility of methods
- remove superfluous code
- make G1CMBitMapClosure pass a HeapWord* instead of a bitmap index,
avoiding that the user code is cluttered with conversions from bitmap
indices to HeapWords
- remove inheritance between G1CMBitMap and G1CMBitMapRO, similar to
the BitMap class make G1CMBitMapRO a "view" of G1CMBitMap.
- remove unused code in G1CMBitMapRO
- move method implementations into .inline.hpp file</pre>
</blockquote>
The changes look good to me.<span class="new"><br>
<br>
<tt>+ return _cm->nextMarkBitMap()->is_marked((HeapWord
*)obj);</tt><br>
<br>
I'd write that as<br>
<br>
<tt>(HeapWord*) obj<br>
<br>
but I'm never quite sure what style is preferable in Hotspot ;-)<br>
<br>
<br>
</tt>Are changes in </span>g1FromCardCache.cpp/.hpp unrelated?<br>
<br>
<blockquote type="cite"
cite="mid:1500024904.3458.8.camel@oracle.com">
<pre wrap="">The next CR JDK-8184347 will deal with moving G1CMBitmap* into separate
files.</pre>
</blockquote>
And while you're at it, you may want to move it to gc/shared and
renamed it to something like MarkBitmap?<br>
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8180193">https://bugs.openjdk.java.net/browse/JDK-8180193</a><br>
<br>
Best regards,<br>
Roman (not official reviewer)<br>
<br>
</body>
</html>