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

[Stefan Karlsson]

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.