[lworld+vector] RFR: Merge lworld

Xiaohong Gong xgong at openjdk.org
Sun Jun 25 04:21:31 UTC 2023


On Sun, 25 Jun 2023 04:13:03 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Merge from lworld in lworld+vector, JDK-8292818 made significant modification to FieldInfo by replacing 96 bits fixed set of metadata fields with UNSIGNED5 compressed fields. 
>> 
>> Other Fixes addressed:-
>> 
>> A) CDS related: 
>>   - All the _VectorPayloadMF*_ classes are part of java.base modules and thus gets archived into a classes.jsa. 
>>   - Code for bookkeeping InlineKlassFixedBlock pointer containing  packing/unpacking adapters, exact bundle size etc. is removed from lworld branch, this causes problem in down stream flow accessing those fields.
>>   
>>   - InstanceKlass::size method takes into account size of InlineKlassFixedBlock and therefore its contents are already part of shared archive, reinitialized the pointer by making an explicit call to _inlineklass_static_block()_ [1] during unshared field restoration.	
>> 
>>   - Java class structure [2] contains primaryType (pointer to primitive klass mirrors) and secondaryTypes (pointer to primitive values mirrors), these are no-longer getting restored during shared archive loading at VM startup. These pointer are used for class level equality comparison. As a workaround, using -Xshare:off flag during regression testing, this will be fixed in a follow up patch. 
>> 
>> B) Removed redundant ResourceMarks during field parsing [3], this was resulting into crashes during re-allocations of  __temp_field_info_ (a GrowableArray) due to __nesting_ level mismatch b/w actual allocation site and re-allocation sites. Re-allocations happens because upfront space reservation for growable array do not take into account multifield bundle size. 
>> 
>> C) JDK-8292818 added a new __index_ field on top regular fields specified in JVM specification (name_index, signature_index, flags_index, access_index) into FieldInfo structure, handled its initialization while parsing multifield[4], injected[5] and user defined fields[6]. 
>> 
>> D) Small change in VectorBox creation routine for proper initialization[7] of _declared_nonstatic_fields before accessing it[8]. 
>> 
>> E) De-optimization related:-
>>   - De-coupled[9] allocation and re-assignment during vector box re-materialization leveraging existing re-assignment framework[10]. This prevents introducing special handlings for non-vector Locations in existing routines since an InlineTypeNode may have scalar / vector / constant inputs (scalarized multifield bundle initialization during make_default [11]).
>> 
>>   - nfield count which determines number of field level bookkeeping at Safe...
>
> src/hotspot/share/prims/vectorSupport.cpp line 283:
> 
>> 281:   jint larval = (jint)*((jint*)&is_larval);
>> 282:   // Vector payload value in an aligned adjacent tuple (8, 16, 32 or 64 bytes).
>> 283:   return allocate_vector_payload_helper(ik, num_elem, elem_bt, larval, THREAD); // safepoint
> 
> I checked method `allocate_vector_payload_helper()`, and I didn't find any fields assignment inside it. Is this right or any further plan to add those assgnment? Besides, I have created a similar fix for the deoptimization: https://github.com/openjdk/valhalla/pull/863. Could you please take a look at it? Maybe we can integrate that change with this merge, or create a separate one if necessary.

Oh, I see the followed calling to `Deoptimization::reassign_fields_by_klass()`. An alternative way is to keep the vector assignment in `vectorSupport.cpp` like before, and other fields reassign in `deoptimization.cpp` like other normal objects. So that we doesn't need to modify too much during deoptimization. The only thing we have to do is passing the secondary fields to safepoint if not vectorized. WDYT?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/866#discussion_r1241024761


More information about the valhalla-dev mailing list