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

[Mikael Gerdin]

On 2015-08-14 11:34, Erik Helin wrote:
> On 2015-08-13, Thomas Schatzl wrote:
>> 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
>
> Thanks for fixing this! Looks good, Reviewed.

Looks good to me too.
/Mikael

>
> Thanks,
> Erik
>
>> Testing:
>> jprt
>>
>> Thanks,
>>    Thomas
>>
>>