RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp

[Christian Thalinger]

On May 20, 2014, at 12:32 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:

> Hi Christian, 
> 
> I added the Reviewed-by line to the changeset I uploaded, so it jchecks fine.
> Is this what you needed? Can I prepare that some more?  

Yes, that’s what I wanted.  Thanks.

> 
> I tested the changeset on linuxx86_64, windowsx86_64, solaris sparc 64 & 32 bit, 
> darwinintel64, linuxppc64 and aix.  Opt & fastdbg build on all (without precompiled headers)
> and dbg build on most of the platforms.
> 
> Thanks for your help,
>  Goetz.
> 
> 
> 
> -----Original Message-----
> From: Christian Thalinger [mailto:christian.thalinger at oracle.com] 
> Sent: Montag, 19. Mai 2014 18:50
> To: Stefan Karlsson
> Cc: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
> 
> Actually, since you are a Commiter can you prepare a changeset so I can just take and push it?  Saves me some time :-)
> 
> On May 19, 2014, at 9:48 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> Alright.  I will push this to hs-comp.
>> 
>> On May 19, 2014, at 8:18 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>> 
>>> On 2014-05-19 16:43, Lindenmaier, Goetz wrote:
>>>> Hi Stefan,
>>>> 
>>>> thanks for the comments!
>>>> I fixed compactibelFreeListSpace.cpp and heapRegion.cpp.
>>>> I also saw another usage of Prefetch in klass.hpp.  I removed
>>>> the macro altogether as it's unused.
>>>> 
>>>> I kept the include in precompiled.hpp though, as for other
>>>> files it's listed there, too.  If you insist, and
>>>> there are no other objections, I'll remove it.
>>> 
>>> OK.
>>> 
>>>> 
>>>> The new webrev is at
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.01
>>> 
>>> Looks good.
>>> 
>>> thanks,
>>> StefanK
>>>> 
>>>> Best regards,
>>>> Goetz.
>>>> 
>>>> -----Original Message-----
>>>> From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
>>>> Sent: Montag, 19. Mai 2014 13:21
>>>> To: Lindenmaier, Goetz; Christian Thalinger
>>>> Cc: hotspot-dev at openjdk.java.net
>>>> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
>>>> 
>>>> Hi Goetz,
>>>> 
>>>> On 2014-05-19 12:58, Lindenmaier, Goetz wrote:
>>>>> Hi Christian,
>>>>> 
>>>>> thanks for the review!
>>>>> 
>>>>> Although prefetch is not used that often, I think it's good
>>>>> that all headers in the os_cpu directories are handled
>>>>> similarly now, i.e., they are only included in the respective
>>>>> shared file.  (For some it's the file in the /cpu/ dir, like
>>>>> copy_linux_x86.inline.hpp.)
>>>>> 
>>>>> Could you sponsor the change?  Do I need a second reviewer?
>>>> Thanks for cleaning this up.
>>>> 
>>>> Some minor comments:
>>>> 
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp.frames.html
>>>> 
>>>> You shouldn't have to include prefetch.inline.hpp from this file.
>>>> 
>>>> 
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/memory/space.inline.hpp.frames.html
>>>> 
>>>> heapRegion.cpp needs to include space.inline.hpp since it uses the
>>>> SCAN_AND_FORWARD macro.
>>>> 
>>>> 
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/precompiled/precompiled.hpp.frames.html
>>>> 
>>>> I don't think its necessary to add the prefetch.inline.hpp to
>>>> precompiled.hpp.
>>>> 
>>>> 
>>>> Otherwise, this looks good to me.
>>>> 
>>>> thanks,
>>>> StefanK
>>>> 
>>>>> Best regards,
>>>>>  Goetz.
>>>>> 
>>>>> 
>>>>> 
>>>>> -----Original Message-----
>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>> Sent: Donnerstag, 15. Mai 2014 17:14
>>>>> To: Lindenmaier, Goetz
>>>>> Cc: hotspot-dev at openjdk.java.net
>>>>> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
>>>>> 
>>>>> Looks good to me.  I thought that prefetch is used more often but apparently not.
>>>>> 
>>>>> On May 15, 2014, at 1:16 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> This change introduces an umbrella header for prefetch_<os>_<cpu>.inline.hpp
>>>>>> files: runtime/prefetch.inline.hpp as proposed in
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-April/013702.html
>>>>>> This follows the scheme applied to atomic.inline.hpp.
>>>>>> 
>>>>>> Further this change adds includes of prefetch.inline.hpp in all .cpp
>>>>>> and .inline.hpp files where a method of Prefetch declared 'inline' is
>>>>>> called.
>>>>>> 
>>>>>> Finally it moves macros calling inline methods of Prefetch from
>>>>>> space.hpp to the corresponding .inline.hpp file and adds the necessary
>>>>>> includes in files using the moved macros.
>>>>>> 
>>>>>> Please review and test this change.  I please need a sponsor.
>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/
>>>>>> 
>>>>>> Thanks and best regards,
>>>>>> Goetz.
>>> 
>> 
>