RFR (M): 8048241: Introduce umbrella header os.inline.hpp and clean up includes

David Holmes david.holmes at oracle.com
Tue Jul 1 11:41:43 UTC 2014

Coleen, Goetz,

This looks good to me too. But it needs to be checked against our closed 
code (I expect changes will be needed there) and we also need to check 
things work okay with and without precompiled header support (the 
solaris build will verify that IIRC).


On 1/07/2014 9:27 PM, Coleen Phillimore wrote:
> Okay, I'll do it.  Since you have a Reviewer, all you need is another
> reviewer (note capitalization).
> Thanks!
> Coleen
> On 7/1/14, 3:29 AM, Lindenmaier, Goetz wrote:
>> Hi Coleen,
>> thanks for the review!
>> I based it on gc, as Stefan pushed my atomic.inline.hpp change
>> into that repo.  Now that change propagated to the other repos,
>> and this one applies nicely (I just checked hs-rt).
>> So I'd appreciate if you sponsor it!  But I still need a second review
>> I guess.
>> Best regards,
>>    Goetz.
>> -----Original Message-----
>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>> Behalf Of Coleen Phillimore
>> Sent: Montag, 30. Juni 2014 18:20
>> To: hotspot-dev at openjdk.java.net
>> Subject: Re: RFR (M): 8048241: Introduce umbrella header os.inline.hpp
>> and clean up includes
>> Goetz,
>> I reviewed this change and it looks great.  Thank you for cleaning this
>> up.  Since it's based on hs-gc repository, I think someone from the GC
>> group should sponsor.  Otherwise, I'd be happy to.
>> Thanks!
>> Coleen
>> (this was my reply to another RFR, sorry)
>> On 6/29/14, 5:00 PM, Lindenmaier, Goetz wrote:
>>> Hi,
>>> This change adds a new header os.inline.hpp including the
>>> os_<os>.include.hpp
>>> headers. This allows to remove around 30 os dependent include
>>> cascades, some of
>>> them even without adding the os.inline.hpp header in that file.
>>> Also, os.inline.hpp is added in several files that call functions
>>> from these
>>> headers where it was missing so far.
>>> Some further cleanups:
>>> OrderAccess include in adaptiveFreeList.cpp is needed because of
>>> freeChunk.hpp.
>>> The include of os.inline.hpp in thread.inline.hpp is needed because
>>> Thread::current() uses thread() from ThreadLocalStorage, which again
>>> uses
>>> os::thread_local_storage_at which is implemented platform dependent.
>>> I moved some methods without dependencies to other .include.hpp files
>>> to os_windows.hpp/os_posix.hpp.  This reduces the need for os.inline.hpp
>>> includes a lot.
>>> Please review and test this change.  I please need a sponsor.
>>> http://cr.openjdk.java.net/~goetz/webrevs/8048241-osInc/webrev.00/
>>> I compiled and tested this without precompiled headers on linuxx86_64,
>>> linuxppc64, windowsx86_64, solaris_sparc64, solaris_sparc32,
>>> darwinx86_64,
>>> aixppc64 in opt, dbg and fastdbg versions.
>>> Thanks and best regards,
>>>     Goetz.

More information about the hotspot-dev mailing list