<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Thomas,<br>
<br>
On 2015-03-16 21:45, Thomas Schatzl wrote:<br>
</div>
<blockquote cite="mid:1426538708.3346.8.camel@oracle.com"
type="cite">
<pre wrap="">Hi,
On Mon, 2015-03-16 at 17:04 +0100, Stefan Karlsson wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi,
Please review this patch to rename and simplify some macros in
specialized_oop_closures.hpp and g1_specialized_oop_closures.hpp.
The patch contains the following changes:
1) The FURTHER_SPECIALIZED* macros are only used by G1, so they have
been renamed into SPECIALIZED_*G1.
2) Removed some unnecessary #ifdefs. The compile will catch the error if
we try to redefine the macro.
3) Gather all CMS specific macros inside a new
SPECIALIZED_OOP_OOP_ITERATE_CLOSURES_CMS macro.
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~stefank/8075247/webrev.01">http://cr.openjdk.java.net/~stefank/8075247/webrev.01</a>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8075247">https://bugs.openjdk.java.net/browse/JDK-8075247</a>
</pre>
</blockquote>
<pre wrap="">
I am not completely sure why in many files touched in this change both
cpp and the corresponding hpp files include
specialized_oop_closures.hpp. Is there a particular reason to do so?</pre>
</blockquote>
<br>
Yes. All files should, preferably, explicitly include the files that
they rely on. Sometimes, that's not done and the dependency is
resolved via the transitive closure of the include files, but
relying on that gives us a more fragile include hierarchy where
seemingly unrelated changes cause compile errors when includes are
changed/removed.<br>
<br>
<br>
One example from this patch: ALL_OOP_OOP_ITERATE_CLOSURES_1 from
specialized_oop_closures.hpp is used by both
instanceClassLoaderKlass.cpp and instanceClassLoaderKlass.hpp:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.cpp.frames.html">http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.cpp.frames.html</a><br>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<br>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
ALL_OOP_OOP_ITERATE_CLOSURES_1(InstanceClassLoaderKlass_OOP_OOP_ITERATE_DEFN)<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.hpp.frames.html">http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.hpp.frames.html</a><br>
<br>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
ALL_OOP_OOP_ITERATE_CLOSURES_1(InstanceClassLoaderKlass_OOP_OOP_ITERATE_DECL)<br>
<br>
<br>
I hope this clarifies why I wanted to add the
specialized_oop_closures.hpp includes in all those files.<br>
<br>
Thanks,<br>
StefanK<br>
<br>
<blockquote cite="mid:1426538708.3346.8.camel@oracle.com"
type="cite">
<pre wrap="">
Thanks,
Thomas
</pre>
</blockquote>
<br>
</body>
</html>