RFR (S): 8133470: Uses of Atomic methods in plab.hpp should be moved to .inline.hpp file

Thomas Schatzl thomas.schatzl at oracle.com
Thu Aug 13 15:32:06 UTC 2015


Hi all,

On Thu, 2015-08-13 at 12:49 +0200, Thomas Schatzl wrote:
> Hi all,
> 
>   in the review for 8073013 one of the review questions was why plab.hpp
> added atomic.inline.hpp, and that that is not good practice.
> 
> This has been a pre-existing bug, i.e. plab.hpp already used the Atomic
> functions that reside in .inline.hpp.
> 
> This CR fixes this issue by moving the problematic calls into
> plab.inline.hpp.
> 
> The only problem is that changing includes from plab.hpp to
> plab.inline.hpp in cpp files and adding plab.hpp in header files opens
> up dependency issues and other missing includes.
> 
> That's why this change is larger than it should.
> 
> Webrev:
> https://bugs.openjdk.java.net/browse/JDK-8133470
> CR:
> http://cr.openjdk.java.net/~tschatzl/8133470/webrev/
> 
> Testing:
> jprt

  Mikael and Erik reminded me that it is not a good idea to move
PLABAllocator::allocate_aligned() into the .cpp file because CMS uses
only allocate_aligned(). This may result in performance degradation for
CMS, which is unwanted.

This required the introduction of a parNewGeneration.inline.hpp for the
method calling it. And some other code shuffling.

Here are new webrevs:
http://cr.openjdk.java.net/~tschatzl/8133470/webrev.0_to_1
http://cr.openjdk.java.net/~tschatzl/8133470/webrev.1

Testing:
jprt

Thanks,
  Thomas





More information about the hotspot-gc-dev mailing list