RFR: 8075247: Cleanup specialized_oop_closures.hpp

Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 17 13:20:31 UTC 2015


Hi,

On Mon, 2015-03-16 at 22:12 +0100, Stefan Karlsson wrote:
> 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.

Okay - I was not aware that this should also be the case for includes by
the header file corresponding to the given cpp file.

Btw, do you know of a tool to find out missing includes (or just find
ones only included transitively)?

Looks good.

Thanks,
  Thomas





More information about the hotspot-gc-dev mailing list