RFR: 8230482: Initial support for empty inline classes

[Frederic Parain]

Ioi,

Thank you for reviewing this change.

new webrev:
http://cr.openjdk.java.net/~fparain/emptyvalue/webrev.02/index.html

I’ve added a comment explaining why inline classes need
two additional slots.

There’s already a test checking that the injected field is not
visible via reflection (EmptyValueTest.java:120-127 in the last
webrev).

I’ve added a few more tests to check that access by reflection to
the empty field in WithEmptyField works. However, this path is
not optimized yet (i.e. new instances of EmptyValue are created).

Regards,

Fred

> On Sep 4, 2019, at 12:39, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> Hi Fred,
> 
> The changes look good to me.
> 
> 1549   const int total_fields = length + num_injected + (is_value_type ? 2 : 0);
> 
> Maybe add a comment what the "2" means?
> 
> Also, I think it will be good to add a Java reflection test to query the fields in
> EmptyValue, to make sure zero fields are returned.
> 
> Thanks
> - Ioi
> 
> 
> On 9/4/19 9:28 AM, Frederic Parain wrote:
>> Remi, Ioi,
>> 
>> Here’s a new webrev with the new tests added:
>> 
>> http://cr.openjdk.java.net/~fparain/emptyvalue/webrev.01/
>> 
>> On a 64bits platform with compressed oops, the layout of
>> the WithEmptyField class is:
>> 
>> Layout of class runtime/valhalla/valuetypes/EmptyValueTest$WithEmptyField
>> | offset | kind | size | alignment | signature | name |
>> Instance fields:
>>   0 RESERVED 12
>>   12 INHERITED 4
>>   16 FLATTENED 1 1 Qruntime/valhalla/valuetypes/EmptyValueTest$EmptyValue; empty
>>   17 EMPTY 3
>>   20 REGULAR 4 4 Ljava/lang/Object; o
>> 
>> The inherited field at offset 12 being the int field from the super class.
>> 
>> Thank you,
>> 
>> Fred
>> 
>> 
>>> On Sep 3, 2019, at 11:41, Ioi Lam <ioi.lam at oracle.com> wrote:
>>> 
>>> Maybe we should also have some System.arraycopy() tests?
>>> 
>>> Thanks
>>> - Ioi
>>> 
>>> On 9/3/19 8:29 AM, Remi Forax wrote:
>>>> Hi Frederic,
>>>> i think you can add a test where an empty inline object is used as field in the middle of several fields
>>>> (perhaps using inheritance to be sure of the layout ?).
>>>> 
>>>> Rémi
>>>> 
>>>> ----- Mail original -----
>>>>> De: "Frederic Parain" <frederic.parain at oracle.com>
>>>>> À: "valhalla-dev" <valhalla-dev at openjdk.java.net>
>>>>> Envoyé: Mardi 3 Septembre 2019 16:24:08
>>>>> Objet: RFR: 8230482: Initial support for empty inline classes
>>>>> Greetings,
>>>>> 
>>>>> Please review this code that adds initial support for empty inline classes.
>>>>> 
>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8230482
>>>>> Webrev: http://cr.openjdk.java.net/~fparain/emptyvalue/webrev.00/index.html
>>>>> 
>>>>> Thank you,
>>>>> 
>>>>> Fred
>