Initial runtime support for the ValueTypes attribute

[Karen Kinnear]

Frederic,

Totally agree with your discussion with Tobias - thanks for that.

Code looks good. Minor questions. I don’t need to see an updated webrev.

instanceKlass.cpp: line 617 “if this class” -> “of this class”
lines 626-6267 “where potential” -> “to detect potential”

classFileParser.cpp lines 4104/4105 
not clear where you are using name and signature in the exception throwing case?

thanks,
Karen

> On May 22, 2018, at 3:00 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
> 
> Karen,
> 
> Good catch on classFileParser lines 4078/4092, the test was wrong,
> but another bug in the way class names were retrieved from the signature
> was hiding the problem. Both are fixed now,
> 
> After a discussion with Tobias, from the JIT compiler team, there’s no need
> to pre-load argument types and return value types for all referenced methods.
> It is sufficient to pre-load those types for the methods the class declares.
> The current implementation is an approximation, as it preloads types for all
> methods of the current class, but it achieves the same semantic.
> 
> New webrev:
> http://cr.openjdk.java.net/~fparain/VTAttribute/webrev.02/index.html
> 
> Thank you,
> 
> Fred
> 
> 
>> On May 18, 2018, at 15:43, Karen Kinnear <KAREN.KINNEAR at ORACLE.COM> wrote:
>> 
>> 
>> Frederic,
>> 
>> Thank you for the improvements. 
>> I agree with not need at this point to do pre-loading for arrays since the element type
>> is already loaded before array creation.
>> I had wondered about pre-loading fields mentioned in field descriptors and agree there
>> is no need.
>> 
>> instanceKlass.cpp line 618 - partial sentence
>> 
>> classFileParser.cpp lines 4078/4092 - did I read these backwards?
>> Isn’t it a ClassFormatError if ACC_FLATTENABLE and NOT is_declared_value_type?
>> 
>> Rest looks good.
>> 
>> thanks,
>> Karen
>> 
>>> On May 18, 2018, at 1:50 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>>> 
>>> Karen,
>>> 
>>> Your comment made me think that I wanted too much
>>> to rush this code out for the JIT team. It needed some
>>> clean up first and more consistency.
>>> 
>>> Regarding arrays, so far, we haven’t identify any array optimization that
>>> would require pre-loading. The element type is loading just before creating
>>> the array, and it is sufficient to implement our current optimization (array flattening)
>>> 
>>> Here’s a new webrev:
>>> http://cr.openjdk.java.net/~fparain/VTAttribute/webrev.01/index.html
>>> 
>>> Changes:
>>> - pre-loading code for flattenable fields in the link phase has been removed:
>>>  this pre-loading is already performed at load time when fields layout is computed.
>>> - error detection and handling has been improved and made consistent between
>>>  pre-loading for flattenable fields and pre-loading for method arguments
>>> - comments have been update
>>> 
>>> To summarize:
>>> - at load time: pre-loading of flattenable fields types
>>> - at link time: pre-loading of method arguments and return types
>>> - at execution time: regular loading of element types for array
>>> 
>>> Error Handling:
>>> - If a field has the ACC_FLATTENABLE flag set but its type is not listed in
>>>  the ValueTypes attribute, an ClassFormatError is thrown
>>> - If a class is pre-loaded but it is not a value class, an ICCE is thrown
>>> 
>>> Thanks,
>>> 
>>> Fred
>>> 
>>> 
>>>> On May 18, 2018, at 10:16, Karen Kinnear <KAREN.KINNEAR at ORACLE.COM> wrote:
>>>> 
>>>> Frederic,
>>>> 
>>>> Code looks really good. Many thanks for doing this so quickly and carefully.
>>>> Thank you for the symbol refcount handling.
>>>> 
>>>> Summary:
>>>> declared fields (static and instance): preload, i.e. classfileparser loads before completing container loading
>>>> linkage to fields and methods, we load at link time prior to creation of vtables/itables
>>>> 
>>>> - this all sounds correct to me.
>>>> 
>>>> instanceKlass.cpp: 621
>>>> - comment is that arrays of value types are not handled
>>>> - do we actually need to preload arrays of value types at link time for any optimizations?
>>>> 
>>>> Empty.java: line 40: Excepted -> Expected
>>>> 
>>>> thanks,
>>>> Karen
>>>> 
>>>>> On May 17, 2018, at 4:04 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>>>>> 
>>>>> Please review this first patch related to the ValueTypes attribute:
>>>>> 
>>>>> http://cr.openjdk.java.net/~fparain/VTAttribute/webrev.00/
>>>>> 
>>>>> The patch includes:
>>>>> - the parsing of the ValueTypes attribute
>>>>> - the creation of meta-data from this attribute
>>>>> - a consistency check between the ACC_FLATTENABLE flag
>>>>> and the ValueTypes attribute
>>>>> - the pre-loading of method arguments types and return values types
>>>>> 
>>>>> Fred
>>>>> 
>>>> 
>>> 
>> 
>