RFR (M): 8048241: Introduce umbrella header os.inline.hpp and clean up includes
Lois Foltan
lois.foltan at oracle.com
Tue Jul 1 11:42:18 UTC 2014
Looks good, minor comment - a large majority of these files need
copyright updates.
Lois
On 7/1/2014 7:27 AM, 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