RFR: 8075247: Cleanup specialized_oop_closures.hpp

[Stefan Karlsson]

Hi Thomas,

On 2015-03-16 21:45, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2015-03-16 at 17:04 +0100, Stefan Karlsson wrote:
>> 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.
>>
>> http://cr.openjdk.java.net/~stefank/8075247/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8075247
> 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?

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.


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:

http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.cpp.frames.html

ALL_OOP_OOP_ITERATE_CLOSURES_1(InstanceClassLoaderKlass_OOP_OOP_ITERATE_DEFN)


http://cr.openjdk.java.net/~stefank/8075247/webrev.01/src/share/vm/oops/instanceClassLoaderKlass.hpp.frames.html

ALL_OOP_OOP_ITERATE_CLOSURES_1(InstanceClassLoaderKlass_OOP_OOP_ITERATE_DECL)


I hope this clarifies why I wanted to add the 
specialized_oop_closures.hpp includes in all those files.

Thanks,
StefanK

>
> Thanks,
>    Thomas
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150316/b2b9f654/attachment.htm>