Initial runtime support for the ValueTypes attribute

Karen Kinnear karen.kinnear at oracle.com
Fri May 18 19:43:06 UTC 2018


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
>>> 
>> 
> 




More information about the valhalla-dev mailing list