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

Stefan Karlsson stefan.karlsson at oracle.com
Mon May 19 11:20:57 UTC 2014


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