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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue May 20 07:32:40 UTC 2014


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?  

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.
>> 
> 



More information about the hotspot-dev mailing list