RFR: 8292818: replace 96-bit representation for field metadata with variable-sized streams [v3]

[Coleen Phillimore]

On Thu, 9 Mar 2023 20:49:04 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> src/hotspot/share/classfile/classFileParser.cpp line 1491:
>> 
>>> 1489:   _temp_field_info = new GrowableArray<FieldInfo>(total_fields);
>>> 1490: 
>>> 1491:   ResourceMark rm(THREAD);
>> 
>> Is the ResourceMark ok here or should it go before allocating _temp_field_info ?
>
> _temp_field_info must survive after ClassFileParser::parse_fields() has returned, so definitively after the allocation of _temp_field_info. That being said, I don't see any reason to have a ResourceMark here, probably a remain of some debugging/tracing code. I'll remove it.

ok, good.  The ResourceMark might be a problem with the GrowableArray if it grows.

>> src/hotspot/share/classfile/classFileParser.cpp line 1608:
>> 
>>> 1606:       fflags.update_injected(true);
>>> 1607:       AccessFlags aflags;
>>> 1608:       FieldInfo fi(aflags, (u2)(injected[n].name_index), (u2)(injected[n].signature_index), 0, fflags);
>> 
>> I don't know why there's a cast here until I read more.  If the FieldInfo name_index and signature_index fields are only u2 sized, could you pass this as an int and then in the constructor assert that the value doesn't overflow u2 instead?
>
> The type of name_index and signature_index is const vmSymbolID, because they names and signatures of injected fields do not come from a constant pool, but from the vmSymbol array.

ok the cast is fine here.

>> src/hotspot/share/oops/fieldStreams.hpp line 104:
>> 
>>> 102:     AccessFlags flags;
>>> 103:     flags.set_flags(field()->access_flags());
>>> 104:     return flags;
>> 
>> Did this used to do this for a reason?
>
> Using the setter rather than the constructor filters out the VM defined flags and keeps only the flags from the class file.

I see, thanks.

-------------

PR: https://git.openjdk.org/jdk/pull/12855