On Tue, 2 Jun 2026 15:02:38 GMT, Matias Saavedra Silva <[email protected]> 
wrote:

>> Classes can be excluded from the CDS archive for a variety of different 
>> reasons, such as having old classfile versions or being stored in a signed 
>> jar. These exclusions can extend to other classes that depend on an excluded 
>> class such as supers or interfaces.
>> 
>> Flat and null-restricted fields have layouts that are calculated when a 
>> classfile is parsed and they rely on the klass of the field being consistent 
>> between dumptime and runtime, thus adding a new dependency. This patch adds 
>> inline flat and null-restricted fields as new dependencies that may lead to 
>> the holding class being excluded from the CDS archive. Verified with a new 
>> test and tier 1-5 tests.
>> 
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Matias Saavedra Silva has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Ioi comments

I haven't been following all of this bug fix but it looks good to me although I 
have minor comment suggestions.

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

> 5487:         TempNewSymbol name = Signature::strip_envelope(sig);
> 5488:         if (name == _class_name) {
> 5489:           // replace the nullptr previously stored now that we have the 
> InstanceKlass for this klass

Suggestion:

          // Replace the nullptr previously stored now that we have the 
InstanceKlass for this klass.

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

> 6334:   // the array of InlineLayoutInfo since it isn't needed, and so it 
> isn't
> 6335:   // transferred to the allocated InstanceKlass.
> 6336:   if (_inline_layout_info_array != nullptr && 
> !(_layout_info->_has_inlined_fields || _has_null_restricted_static_fields)) {

Suggestion:

  if (_inline_layout_info_array != nullptr && 
!_layout_info->_has_inlined_fields &&  !_has_null_restricted_static_fields)) {


Was there a reason to group the last two restrictions?

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

> 6396:         } else {
> 6397:           // dummy setting to triggger the allocation of the 
> inline_layout_info array
> 6398:           // the real pointer will be set later in 
> ::fill_instance_klass, once the InlineKlass has been allocated

Suggestion:

          // Dummy setting to trigger the allocation of the inline_layout_info 
array -
          // the real pointer will be set later in fill_instance_klass, once 
the InlineKlass has been allocated.

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

PR Review: 
https://git.openjdk.org/valhalla/pull/2464#pullrequestreview-4413104530
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2464#discussion_r3344052865
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2464#discussion_r3344013365
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2464#discussion_r3343976979

Reply via email to