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

[Christian Thalinger]

You’re welcome.

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

> Hi Christian, 
> 
> thanks for pushing the change!
> 
> Best regards,
>  Goetz.
> 
> -----Original Message-----
> From: Christian Thalinger [mailto:christian.thalinger at oracle.com] 
> Sent: Dienstag, 20. Mai 2014 17:17
> To: Lindenmaier, Goetz
> Cc: Stefan Karlsson; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
> 
> 
> 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.
>>>> 
>>> 
>> 
>