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