RFR(S): 8190432 [MVT] Value Types should have a default pre-allocated

Frederic Parain frederic.parain at oracle.com
Thu Nov 2 13:40:32 UTC 2017


Tobias,

Thank you for the review.
Indentation fixed and changeset pushed.

Fred

On 11/02/2017 06:30 AM, Tobias Hartmann wrote:
> Hi Fred,
> 
> On 01.11.2017 16:34, Frederic Parain wrote:
>> http://cr.openjdk.java.net/~fparain/8190432/webrev.01/index.html
> 
> Looks good to me!
> 
> There's an indentation problem in line 5771 of classFileParser.cpp (no need to send another webrev).
> 
> All tests pass with my fix for 8190458 on top.
> 
> Thanks,
> Tobias
> 
>>> On Nov 1, 2017, at 09:35, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>>
>>> Hi Fred,
>>>
>>> just hit this build problem on JPRT when trying another patch on top:
>>>
>>> /opt/jprt/T/P1/120645.tobias/s/src/hotspot/share/memory/oopFactory.cpp: In static member function 'static arrayOopDesc* oopFactory::new_valueArray(Klass*, int, Thread*)':
>>> /opt/jprt/T/P1/120645.tobias/s/src/hotspot/share/memory/oopFactory.cpp:112:45: error: invalid conversion from 'oop' to 'instanceOop' [-fpermissive]
>>>     instanceOop value = vklass->default_value();
>>>
>>> Thanks,
>>> Tobias
>>>
>>> On 01.11.2017 09:15, Tobias Hartmann wrote:
>>>> Hi Fred,
>>>> this looks good to me! Some style comments:
>>>> classFileParser.cpp
>>>> - line 1724: whitespace between "index" and "++"
>>>> - line 5770: you can merge the else and the if below
>>>> interpreterRuntime.cpp
>>>> - line 220 and 359: Wouldn't it make sense to move the asserts into ValueKlass::default_value()? Because right now you are only checking at vwithfield and vdefault.
>>>> I've filed JDK-8190458 for the C2 changes.
>>>> Best regards,
>>>> Tobias
>>>> On 31.10.2017 18:35, Frederic Parain wrote:
>>>>> Please review this small changeset to pre-allocate a default value:
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8190432
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~fparain/8190432/webrev.00/index.html
>>>>>
>>>>> The changeset injects a static field into each Java mirror of a
>>>>> value type to keep a reference to the pre-allocated value.
>>>>>
>>>>> All hotspot_valhalla tests pass.
>>>>>
>>>>> Thank you,
>>>>>
>>>>> Fred
>>



More information about the valhalla-dev mailing list