<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Goetz,<br>
<br>
Thanks for cleaning this up!<br>
<br>
You'll have some merge conflicts with:<br>
8146690: Make all classes in GC follow the naming convention<br>
<br>
So we might want to coordinate that somehow.<br>
<br>
Inlined:<br>
<br>
<div class="moz-cite-prefix">On 2016-01-13 23:54, Lindenmaier, Goetz
wrote:<br>
</div>
<blockquote
cite="mid:4295855A5C1DE049A61835A1887419CC41F141D0@DEWDFEMB12A.global.corp.sap"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNormal">Hi <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">This change finishes the cleanup started in
"8146401: Clean up oop.hpp: add inline directives and fix
header files"<o:p></o:p></p>
<p class="MsoNormal">by fixing the issues in gc files. See also
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8146395">
https://bugs.openjdk.java.net/browse/JDK-8146395</a> and the
discussion <o:p></o:p></p>
<p class="MsoNormal">of this change in the rt mailing list: <a
moz-do-not-send="true"
href="http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-January/017440.html"><a class="moz-txt-link-freetext" href="http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-January/017440.html">http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-January/017440.html</a></a><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Most issues can be fixed by including the
proper .inline.hpp header or moving a function there.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I added new files
src/share/vm/gc/shared/referenceProcessor.inline.hpp and
g1SATBCardTableModRefBS.inline.hpp.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I moved the following methods to their .cpp
files. In case someone considers these<o:p></o:p></p>
<p class="MsoNormal">performance relevant, I will add an
inline.hpp file for them:<o:p></o:p></p>
<p class="MsoNormal"> PromotedObject::next and ::setNext
--> promotionInfo.cpp<o:p></o:p></p>
<p class="MsoNormal"> ObjectStartArray::object_start
--> objectStartArray.cpp<o:p></o:p></p>
<p class="MsoNormal"> FillClosure::do_addr to
--> psParallelCompact.cpp (ok, only used here)<o:p></o:p></p>
<p class="MsoNormal"> ageTable::add
--> ageTable.cpp<o:p></o:p></p>
<p class="MsoNormal"> KlassSizeStats::count_array / ::count
--> heapInspection.cpp<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Please review this change. I please need a
sponsor. <o:p></o:p></p>
<p class="MsoNormal"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Egoetz/wr16/8146395-oopInline-gc/webrev.01/">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/</a></p>
</div>
</blockquote>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/cms/cmsOopClosures.inline.hpp.frames.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/cms/cmsOopClosures.inline.hpp.frames.html</a><br>
<br>
Why did you move trim_queues?<br>
<br>
I'm personally not particularly fond of these macro constructs,
since the makes it harder to navigate the code in IDEs.<br>
88 DO_OOP_WORK_NV_IMPL(Par_PushOrMarkClosure)<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/g1/g1SATBCardTableModRefBS.hpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/g1/g1SATBCardTableModRefBS.hpp.udiff.html</a><br>
<br>
// These are the more general virtual versions.<br>
- virtual void write_ref_field_pre_work(oop* field, oop new_val) {<br>
- inline_write_ref_field_pre(field, new_val);<br>
- }<br>
- virtual void write_ref_field_pre_work(narrowOop* field, oop
new_val) {<br>
- inline_write_ref_field_pre(field, new_val);<br>
- }<br>
+ inline virtual void write_ref_field_pre_work(oop* field, oop
new_val);<br>
+ inline virtual void write_ref_field_pre_work(narrowOop* field,
oop new_val);<br>
<br>
I'm not sure how this is going to work. Have you verified that
BarrierSet::write_ref_field_pre works when called while running G1?<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.cpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.cpp.udiff.html</a><br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.hpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/objectStartArray.hpp.udiff.html</a><br>
<br>
I think object_start should be moved to an inline.hpp file, or you
need to verify that this has no performance impact.<br>
<br>
---<br>
<br>
-#undef assert_covered_region_contains<br>
<br>
^ This seems like an unrelated change.<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/psScavenge.hpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/parallel/psScavenge.hpp.udiff.html</a><br>
<br>
There's no need for this function to be inlined.<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/shared/ageTable.cpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/gc/shared/ageTable.cpp.udiff.html</a><br>
<br>
Move to inline.hpp file.<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/oops/oop.hpp.udiff.html">http://cr.openjdk.java.net/~goetz/wr16/8146395-oopInline-gc/webrev.01/src/share/vm/oops/oop.hpp.udiff.html</a><br>
<br>
Spurious new-line<br>
<br>
+ inline uint age() const;<br>
inline void incr_age();<br>
<br>
+<br>
// mark-sweep support<br>
void follow_body(int begin, int end);<br>
<br>
Thanks,<br>
StefanK<br>
<br>
<blockquote
cite="mid:4295855A5C1DE049A61835A1887419CC41F141D0@DEWDFEMB12A.global.corp.sap"
type="cite">
<div class="WordSection1">
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Best regards,<o:p></o:p></p>
<p class="MsoNormal"> Goetz Lindenmaier<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">(The webrev messed up some change comments,
the diffs and patch are fine, though.)<o:p></o:p></p>
</div>
</blockquote>
<br>
</body>
</html>