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

[David Holmes]

On Fri, 3 Mar 2023 14:50:34 GMT, Frederic Parain <fparain at openjdk.org> wrote:

> Please review this change re-implementing the FieldInfo data structure.
> 
> The FieldInfo array is an old data structure storing fields metadata. It has poor extension capabilities, a complex management code because of lack of strong typing and semantic overloading, and a poor memory efficiency.
> 
> The new implementation uses a compressed stream to store those metadata, achieving better memory density and providing flexible extensibility, while exposing a strongly typed set of data when uncompressed. The stream is compressed using the unsigned5 encoding, which alreay present in the JDK (because of pack200) and the JVM (because JIT compulers use it to comrpess debugging information).
> 
> More technical details are available in the CR: https://bugs.openjdk.org/browse/JDK-8292818
> 
> Those changes include a re-organisation of fields' flags, splitting the previous heterogeneous AccessFlags field into three distincts flag categories: immutable flags from the class file, immutable fields defined by the JVM, and finally mutable flags defined by the JVM.
> 
> The SA, CI, and JVMCI, which all used to access the old FieldInfo array, have been updated too to deal with the new FieldInfo format.
> 
> Tested with mach5, tier 1 to 7.
> 
> Thank you.

HI Fred,

I've taken one pass through this but it is a huge set of changes to try and digest. At this stage just a bunch of style nits.

Thanks.

src/hotspot/share/classfile/classFileParser.cpp line 1632:

> 1630:     {
> 1631:       debug_only(NoSafepointVerifier nsv;)
> 1632:       for(int i = 0; i < _temp_field_info->length(); i++) {

Nit: space after 'for' please

src/hotspot/share/classfile/classFileParser.cpp line 2012:

> 2010: void ClassFileParser::FieldAnnotationCollector::apply_to(FieldInfo* f) {
> 2011:   if (is_contended())
> 2012:   // Setting the contended group also set the contended bit in field flags

Nit: s/set/sets/

src/hotspot/share/oops/fieldInfo.cpp line 30:

> 28: 
> 29: void FieldInfo::print(outputStream* os, ConstantPool* cp) {
> 30:   os->print_cr("index=%d name_index=%d name=%s signature_index=%d signature=%s offset=%d AccessFlags=%d FieldFlags=%d initval_index=%d gen_signature_index=%d, gen_signature=%s contended_group=%d",

Nit: please break up this line

src/hotspot/share/oops/fieldInfo.cpp line 120:

> 118:   *java_fields_count = r.next_uint();
> 119:   *injected_fields_count = r.next_uint();
> 120:   while(r.has_next()) {

Nit: space before (

src/hotspot/share/oops/fieldInfo.cpp line 135:

> 133:   int java_field_count = r.next_uint();
> 134:   int injected_fields_count = r.next_uint();
> 135:   while(r.has_next()) {

Nit: space before (

src/hotspot/share/oops/fieldInfo.cpp line 140:

> 138:     fi.print(os, cp);
> 139:   }
> 140: }

Newline needed at EOF

src/hotspot/share/oops/fieldInfo.inline.hpp line 135:

> 133:     new_flags = old_flags | mask;
> 134:     witness = Atomic::cmpxchg(&flags, old_flags, new_flags);
> 135:   } while(witness != old_flags);

Nit: space before (

src/hotspot/share/oops/fieldInfo.inline.hpp line 155:

> 153: inline void FieldStatus::update_access_watched(bool z) { update_flag(_fs_access_watched, z); }
> 154: inline void FieldStatus::update_modification_watched(bool z) { update_flag(_fs_modification_watched, z); }
> 155: inline void FieldStatus::update_initialized_final_update(bool z) {update_flag(_initialized_final_update, z); }

Nit: space after {

src/hotspot/share/oops/fieldStreams.hpp line 43:

> 41:  protected:
> 42:   const Array<u1>* _fieldinfo_stream;
> 43:   FieldInfoReader   _reader;

Nit variable name alignment seems off

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

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