RFR 8153646: Move vm/utilities/array.hpp to vm/oops

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8153646: Move vm/utilities/array.hpp to vm/oops

harold seigel
Hi,

Please review this JDK-10 change to move hotspot header file array.hpp
from the vm/utilities directory to the vm/oops directory.  This was done
because after moving typedefs for basic type arrays to
growableArray.hpp, the only remaining declaration in array.hpp is the
metaspace class Array.

Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8153646/webrev/

JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8153646

The fix was tested with JCK Lang and VM tests, the JTreg hotspot,
java/io, java/lang, java/util and other tests, the co-located NSK tests,
and with JPRT.

Thanks, Harold

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8153646: Move vm/utilities/array.hpp to vm/oops

serguei.spitsyn@oracle.com
Hi Harold,

The fix looks good.

Thanks,
Serguei


On 5/9/17 10:39, harold seigel wrote:

> Hi,
>
> Please review this JDK-10 change to move hotspot header file array.hpp
> from the vm/utilities directory to the vm/oops directory.  This was
> done because after moving typedefs for basic type arrays to
> growableArray.hpp, the only remaining declaration in array.hpp is the
> metaspace class Array.
>
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8153646/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8153646
>
> The fix was tested with JCK Lang and VM tests, the JTreg hotspot,
> java/io, java/lang, java/util and other tests, the co-located NSK
> tests, and with JPRT.
>
> Thanks, Harold
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8153646: Move vm/utilities/array.hpp to vm/oops

David Holmes
In reply to this post by harold seigel
Hi Harold,

On 10/05/2017 3:39 AM, harold seigel wrote:
> Hi,
>
> Please review this JDK-10 change to move hotspot header file array.hpp
> from the vm/utilities directory to the vm/oops directory.  This was done
> because after moving typedefs for basic type arrays to
> growableArray.hpp, the only remaining declaration in array.hpp is the
> metaspace class Array.
>
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8153646/webrev/

Looks good - two minor comments:

Can I ask a favour - for these modified "empty" files:

src/cpu/aarch64/vm/c1_FpuStackSim_aarch64.cpp
src/cpu/arm/vm/c1_FpuStackSim_arm.cpp
src/cpu/sparc/vm/c1_FpuStackSim_sparc.cpp

can you delete all of the #include lines, please.

---

src/share/vm/c1/c1_CodeStubs.hpp

This uses GrowableArray so should #include its header.

Thanks,
David


> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8153646
>
> The fix was tested with JCK Lang and VM tests, the JTreg hotspot,
> java/io, java/lang, java/util and other tests, the co-located NSK tests,
> and with JPRT.
>
> Thanks, Harold
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8153646: Move vm/utilities/array.hpp to vm/oops

harold seigel
In reply to this post by serguei.spitsyn@oracle.com
Thanks Serguei!

Harold


On 5/9/2017 6:37 PM, [hidden email] wrote:

> Hi Harold,
>
> The fix looks good.
>
> Thanks,
> Serguei
>
>
> On 5/9/17 10:39, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-10 change to move hotspot header file
>> array.hpp from the vm/utilities directory to the vm/oops directory.  
>> This was done because after moving typedefs for basic type arrays to
>> growableArray.hpp, the only remaining declaration in array.hpp is the
>> metaspace class Array.
>>
>> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8153646/webrev/
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8153646
>>
>> The fix was tested with JCK Lang and VM tests, the JTreg hotspot,
>> java/io, java/lang, java/util and other tests, the co-located NSK
>> tests, and with JPRT.
>>
>> Thanks, Harold
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8153646: Move vm/utilities/array.hpp to vm/oops

harold seigel
In reply to this post by David Holmes
Thanks David!

I'll make those changes before pushing it.

Harold


On 5/10/2017 12:27 AM, David Holmes wrote:

> Hi Harold,
>
> On 10/05/2017 3:39 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-10 change to move hotspot header file array.hpp
>> from the vm/utilities directory to the vm/oops directory.  This was done
>> because after moving typedefs for basic type arrays to
>> growableArray.hpp, the only remaining declaration in array.hpp is the
>> metaspace class Array.
>>
>> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8153646/webrev/
>
> Looks good - two minor comments:
>
> Can I ask a favour - for these modified "empty" files:
>
> src/cpu/aarch64/vm/c1_FpuStackSim_aarch64.cpp
> src/cpu/arm/vm/c1_FpuStackSim_arm.cpp
> src/cpu/sparc/vm/c1_FpuStackSim_sparc.cpp
>
> can you delete all of the #include lines, please.
>
> ---
>
> src/share/vm/c1/c1_CodeStubs.hpp
>
> This uses GrowableArray so should #include its header.
>
> Thanks,
> David
>
>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8153646
>>
>> The fix was tested with JCK Lang and VM tests, the JTreg hotspot,
>> java/io, java/lang, java/util and other tests, the co-located NSK tests,
>> and with JPRT.
>>
>> Thanks, Harold
>>