RFR: 8258459: Decouple gc_globals.hpp from globals.hpp

Coleen Phillimore coleenp at openjdk.java.net
Tue Dec 22 13:11:56 UTC 2020


On Wed, 16 Dec 2020 06:24:59 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Currently, [globals.hpp](https://github.com/openjdk/jdk/blob/17ace8339dd8235f3811e3975d9ccc77910b0c77/src/hotspot/share/runtime/globals.hpp#L29) is including gc_globals.hpp. This exposes all the GC command-line flags to almost all cpp files. However, only about 1/3 of the cpp files would require gc_globals.hpp.
> 
> This RFE improves modularity and HotSpot build time.
> 
> **Review notes:**
> - Please start with globals.hpp, gc_globals.hpp.
> - Flags related to TLAB are frequently used by other header files. I moved these flags to a new header tlab_globals.hpp.
> - Some tweaking of oop.hpp to avoid including gc_globals.hpp in this popular header file. (Otherwise gc_globals.hpp would be included by everyone).
> - The other changes are just files that use GC flags. They should have included gc_globals.hpp but didn't.
> 
> **Testing:**
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

Looks good!  I had a couple of minor change requests.

src/hotspot/share/oops/oop.inline.hpp line 187:

> 185:       // disjunct below to fail if the two comparands are computed across such
> 186:       // a concurrent change.
> 187:       assert((s == klass->oop_size(this)) ||

It might be nicer to throw the second half of this assert into the .cpp file, like
assert(s == klass->oop_size(this) || gc_has_forwarded(), "wrong array object size");
With the special comment above it in the cpp file.  Then someone has the potential to add more information if the assert fails, and that gets Universe::hpp out of the inline file (if that helps with future cleanups).

src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp line 61:

> 59: }
> 60: 
> 61: size_t ThreadLocalAllocBuffer::initial_refill_waste_limit()            { return desired_size() / TLABRefillWasteFraction; }

nit: please line this up.

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1797


More information about the hotspot-dev mailing list