Initial runtime support for the ValueTypes attribute
frederic.parain at oracle.com
Tue May 22 19:00:45 UTC 2018
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.
> On May 18, 2018, at 15:43, Karen Kinnear <KAREN.KINNEAR at ORACLE.COM> wrote:
> 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.
>> On May 18, 2018, at 1:50 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>> 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:
>> - 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
>>> On May 18, 2018, at 10:16, Karen Kinnear <KAREN.KINNEAR at ORACLE.COM> wrote:
>>> Code looks really good. Many thanks for doing this so quickly and carefully.
>>> Thank you for the symbol refcount handling.
>>> 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
>>>> 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:
>>>> 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
More information about the valhalla-dev