RFR: 8192061: Clean up allocation.inline.hpp includes

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8192061: Clean up allocation.inline.hpp includes

Stefan Karlsson
Hi all,

Please review this patchset to clean up our includes of
allocation.inline.hpp and move usages of its functions out from header
files into .cpp and .inline.hpp files.

http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all

I've split this patch into multiple separate patches and tried to group
them according to HotSpot area, to make it easier to review. I can also
create separate bugs for this, if that's preferable.

GC:
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/

RT:
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/

SHARED:
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/

Thanks,
StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192061: Clean up allocation.inline.hpp includes

coleen.phillimore

Hi Stefan,  I looked at the whole review and it's not hard to review.  I
love these sort of changes.

Have you tested this on all platforms that we support?  I was wondering
if SAP should do the same.  It seems like sort of a big change to get in
by Friday but not impossible.

Thanks,
Coleen

On 11/29/17 6:21 AM, Stefan Karlsson wrote:

> Hi all,
>
> Please review this patchset to clean up our includes of
> allocation.inline.hpp and move usages of its functions out from header
> files into .cpp and .inline.hpp files.
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all
>
> I've split this patch into multiple separate patches and tried to
> group them according to HotSpot area, to make it easier to review. I
> can also create separate bugs for this, if that's preferable.
>
> GC:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/ 
>
>
> RT:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/
>
> SHARED:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/ 
>
>
> Thanks,
> StefanK

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192061: Clean up allocation.inline.hpp includes

Erik Österlund-2
In reply to this post by Stefan Karlsson
Hi Stefan,

Nice. Noticed though that in sharedPathsMiscInfo.hpp, it seems like you
accidentally (I assume) added an include of allocation.inline.hpp.
http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/src/hotspot/share/classfile/sharedPathsMiscInfo.hpp.sdiff.html

Thanks,
/Erik

On 2017-11-29 12:21, Stefan Karlsson wrote:

> Hi all,
>
> Please review this patchset to clean up our includes of
> allocation.inline.hpp and move usages of its functions out from header
> files into .cpp and .inline.hpp files.
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all
>
> I've split this patch into multiple separate patches and tried to
> group them according to HotSpot area, to make it easier to review. I
> can also create separate bugs for this, if that's preferable.
>
> GC:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/ 
>
>
> RT:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/
>
> SHARED:
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/ 
>
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/ 
>
>
> Thanks,
> StefanK

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192061: Clean up allocation.inline.hpp includes

Stefan Karlsson
Hi Erik,

On 2017-11-29 14:35, Erik Österlund wrote:
> Hi Stefan,
>
> Nice. Noticed though that in sharedPathsMiscInfo.hpp, it seems like you
> accidentally (I assume) added an include of allocation.inline.hpp.
> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/src/hotspot/share/classfile/sharedPathsMiscInfo.hpp.sdiff.html 

Yes, you are right. I used to have the patches in a different order and
accidentally left/introduced this include.

I'll rerun my build tests with this removed.

Thanks,
StefanK

>
>
> Thanks,
> /Erik
>
> On 2017-11-29 12:21, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patchset to clean up our includes of
>> allocation.inline.hpp and move usages of its functions out from header
>> files into .cpp and .inline.hpp files.
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all
>>
>> I've split this patch into multiple separate patches and tried to
>> group them according to HotSpot area, to make it easier to review. I
>> can also create separate bugs for this, if that's preferable.
>>
>> GC:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/ 
>>
>>
>> RT:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/
>>
>> SHARED:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/ 
>>
>>
>> Thanks,
>> StefanK
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192061: Clean up allocation.inline.hpp includes

Stefan Karlsson
In reply to this post by coleen.phillimore
Hi Coleen,

On 2017-11-29 14:15, [hidden email] wrote:
>
> Hi Stefan,  I looked at the whole review and it's not hard to review.  I
> love these sort of changes.

Thanks!

>
> Have you tested this on all platforms that we support?

Yes, and I've also built open-only bits.

   I was wondering
> if SAP should do the same.  It seems like sort of a big change to get in
> by Friday but not impossible.

I think this should go in as soon as possible.

Thanks,
StefanK

>
> Thanks,
> Coleen
>
> On 11/29/17 6:21 AM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patchset to clean up our includes of
>> allocation.inline.hpp and move usages of its functions out from header
>> files into .cpp and .inline.hpp files.
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all
>>
>> I've split this patch into multiple separate patches and tried to
>> group them according to HotSpot area, to make it easier to review. I
>> can also create separate bugs for this, if that's preferable.
>>
>> GC:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/ 
>>
>>
>> RT:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/
>>
>> SHARED:
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/ 
>>
>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/ 
>>
>>
>> Thanks,
>> StefanK
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192061: Clean up allocation.inline.hpp includes

coleen.phillimore


On 11/29/17 9:03 AM, Stefan Karlsson wrote:

> Hi Coleen,
>
> On 2017-11-29 14:15, [hidden email] wrote:
>>
>> Hi Stefan,  I looked at the whole review and it's not hard to
>> review.  I love these sort of changes.
>
> Thanks!
>
>>
>> Have you tested this on all platforms that we support?
>
> Yes, and I've also built open-only bits.
>
>   I was wondering
>> if SAP should do the same.  It seems like sort of a big change to get
>> in by Friday but not impossible.
>
> I think this should go in as soon as possible.

Okay with me.  Thanks for all the testing.
Coleen

>
> Thanks,
> StefanK
>
>>
>> Thanks,
>> Coleen
>>
>> On 11/29/17 6:21 AM, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patchset to clean up our includes of
>>> allocation.inline.hpp and move usages of its functions out from
>>> header files into .cpp and .inline.hpp files.
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.all
>>>
>>> I've split this patch into multiple separate patches and tried to
>>> group them according to HotSpot area, to make it easier to review. I
>>> can also create separate bugs for this, if that's preferable.
>>>
>>> GC:
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.00.GC.g1ConcurrentRefine/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.01.GC.spaceCounters/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.02.GC.g1ParScanThreadState.hpp/ 
>>>
>>>
>>> RT:
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.03.GC.adaptivePaddedAverage/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.04.RT.diagnosticArgument/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.05.RT.methodHandles/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.06.RT.constantPool/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.07.RT.sharedPathsMiscInfo/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.08.RT.generateOopMap/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.09.RT.objectMonitor/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.10.RT.arguments/
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.11.RT.osThread/
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.12.resourceArea/
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.13.RT.array/
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.14.RT.test_logMessage/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.15.RT.park/
>>>
>>> SHARED:
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.16.SHARED.growableArray/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.17.SHARED.removeIncludes/ 
>>>
>>> http://cr.openjdk.java.net/~stefank/8192061/webrev.01.18.SHARED.precompiled/ 
>>>
>>>
>>> Thanks,
>>> StefanK
>>