8384756: [lworld] Exclude class from archive if class of inline field is excluded by matias9927 · Pull Request #2464 · openjdk/valhalla

matias9927 · 2026-05-21T23:09:11Z

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.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8384756: [lworld] Exclude class from archive if class of inline field is excluded (Bug - P2)

Reviewers

Ioi Lam (@iklam - Committer) ⚠️ Review applies to e723f009
Frederic Parain (@fparain - Committer)
Dan Heidinga (@DanHeidinga - no project role)

Contributors

Frederic Parain <fparain@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2464/head:pull/2464
$ git checkout pull/2464

Update a local copy of the PR:
$ git checkout pull/2464
$ git pull https://git.openjdk.org/valhalla.git pull/2464/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2464

View PR using the GUI difftool:
$ git pr show -t 2464

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2464.diff

Using Webrev

Link to Webrev Comment

…is excluded

bridgekeeper · 2026-05-21T23:10:38Z

👋 Welcome back matsaave! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-05-21T23:12:49Z

@matias9927 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8384756: [lworld] Exclude class from archive if class of inline field is excluded

Co-authored-by: Frederic Parain <fparain@openjdk.org>
Reviewed-by: iklam, fparain, heidinga

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1 new commit pushed to the lworld branch:

2cbfd78: 8385928: [lworld] Clean up some redundant core libraries diffs

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

mlbridge · 2026-05-22T16:09:56Z

Webrevs

fparain · 2026-05-22T16:47:55Z

+          Thread* current = Thread::current();
+          Handle loader(current, k->class_loader());
+          Symbol* field_klass_name = Signature::strip_envelope(fs.signature());
+          Klass* field_klass = SystemDictionary::find_instance_or_array_klass(current, field_klass_name, loader);


Why doing a system dictionary search for flat fields when a pointer is already directly available in the _inline_layout_info_array?

if the field is a null_free_inline_type is it also available in the _inline_layout_info_array?
This search is pretty quick so would be good to have them done the same way.

I was hoping the klasses were cached somewhere but I forgot about that array. Thanks for pointing it out!

null-restricted fields are not in the _inline_layout_info_array, but they should be rarer (@NullRestricted is not an officially supported annotation). We don't know yet what the future of null-restricted field will look like, so I would not add them to the _inline_layout_info-array for now.

Update: fields marked as null-restricted at CDS dump time are in fact in the _inline_layout_info_array because the class file parser fills the array with all known inline classes, and the @NullRestricted annotation is kept only for known value classes.

fparain · 2026-05-29T14:29:44Z

Here's a patch that addresses the issue with null-restricted static fields: it ensures that by the time the class is loaded, all static fields with a @NullRestricted annotation have been validated, and a pointer to the InlineKlass of their type has been set in the inline_layout_info_array.
nr_patch.patch

matias9927 · 2026-06-01T17:08:06Z

/contributor add @fparain

openjdk · 2026-06-01T17:09:41Z

@matias9927
Contributor Frederic Parain <fparain@openjdk.org> successfully added.

iklam

CDS changes look good to me. Some nits.

DanHeidinga · 2026-06-02T19:01:25Z

+    // Inline fields need to have their layouts preserved between dumptime and runtime.
+    // To ensure this, the types of the fields must be stored in the archive along with
+    // the field holder.
+    if (k->has_inlined_fields()) {


@fparain were you expecting to see an updated if statement with two conditions here?

if (k->has_inlined_fields() || k->has_null_restricted_static_fields()) {...}

Yes, null restricted static fields must be in the candidate list along with the inlined fields.

coleenp

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

openjdk · 2026-06-04T12:48:05Z

@matias9927 this pull request can not be integrated into lworld due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout signed_jar_8384756
git fetch https://git.openjdk.org/valhalla.git lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push

fparain

Looks good to me.

matias9927 · 2026-06-04T16:31:50Z

Thanks for the reviews, comments, and discussions @coleenp @fparain @DanHeidinga and @iklam!
/integrate

openjdk · 2026-06-04T17:20:35Z

Going to push as commit 022f70b.
Since your change was applied there has been 1 commit pushed to the lworld branch:

2cbfd78: 8385928: [lworld] Clean up some redundant core libraries diffs

Your commit was automatically rebased without conflicts.

openjdk · 2026-06-04T17:20:58Z

@matias9927 Pushed as commit 022f70b.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

matias9927 added 2 commits May 21, 2026 18:53

8384756: [lworld] Exclude class from archive if class of inline type …

338d8c5

…is excluded

Cleanup

711f88c

matias9927 changed the title ~~[lworld] Exclude class from archive if class of inline type is excluded~~ 8384756: [lworld] Exclude class from archive if class of inline type is excluded May 21, 2026

matias9927 changed the title ~~8384756: [lworld] Exclude class from archive if class of inline type is excluded~~ 8384756: [lworld] Exclude class from archive if class of inline field is excluded May 21, 2026

matias9927 added 2 commits May 22, 2026 11:47

Added test

72feb7b

Fixed copyrights

cb60cc6

matias9927 marked this pull request as ready for review May 22, 2026 16:04

openjdk Bot added the rfr Pull request is ready for review label May 22, 2026

fparain reviewed May 22, 2026

View reviewed changes

Fixed test, removed from group, Fred comments

99d6b07

iklam reviewed May 22, 2026

View reviewed changes

Comment thread test/hotspot/jtreg/runtime/cds/appcds/test-classes/excluded/ExcludedValueClass.java Outdated

Comment thread test/hotspot/jtreg/runtime/cds/appcds/InlineFieldExclusionTest.java

Comment thread test/hotspot/jtreg/runtime/cds/appcds/test-classes/InlineFieldExclusionApp.java

Ioi comments

d567f8d

Fred patch

068db09

fparain reviewed Jun 1, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

DanHeidinga reviewed Jun 1, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

Fred and Dan comments

db79c95

DanHeidinga reviewed Jun 1, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

DanHeidinga reviewed Jun 1, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

Removed fastpath for statics

c04eec1

iklam reviewed Jun 2, 2026

View reviewed changes

Comment thread test/hotspot/jtreg/runtime/cds/appcds/InlineFieldExclusionTest.java Outdated

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

Ioi comments

e723f00

iklam approved these changes Jun 2, 2026

View reviewed changes

openjdk Bot added the ready Pull request is ready to be integrated label Jun 2, 2026

DanHeidinga reviewed Jun 2, 2026

View reviewed changes

coleenp reviewed Jun 2, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/classFileParser.cpp Outdated

Comment thread src/hotspot/share/classfile/classFileParser.cpp

Comment thread src/hotspot/share/classfile/classFileParser.cpp Outdated

fparain reviewed Jun 3, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/systemDictionaryShared.cpp Outdated

Include null-restricted static fields and added test

2d9fe73

openjdk Bot added merge-conflict Pull request has merge conflict with target branch and removed ready Pull request is ready to be integrated labels Jun 4, 2026

DanHeidinga reviewed Jun 4, 2026

View reviewed changes

Comment thread src/hotspot/share/classfile/classFileParser.hpp Outdated

matias9927 added 2 commits June 4, 2026 10:17

Added new test app

6ea84ea

Merge branch 'lworld' into signed_jar_8384756

edd3153

openjdk Bot added ready Pull request is ready to be integrated and removed merge-conflict Pull request has merge conflict with target branch labels Jun 4, 2026

Dan comment

1e025c1

fparain approved these changes Jun 4, 2026

View reviewed changes

DanHeidinga approved these changes Jun 4, 2026

View reviewed changes

openjdk Bot added the integrated Pull request has been integrated label Jun 4, 2026

openjdk Bot closed this Jun 4, 2026

openjdk Bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 4, 2026

Conversation

matias9927 commented May 21, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper Bot commented May 21, 2026

Uh oh!

openjdk Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

fparain May 22, 2026

Choose a reason for hiding this comment

Uh oh!

coleenp May 22, 2026

Choose a reason for hiding this comment

Uh oh!

matias9927 May 22, 2026

Choose a reason for hiding this comment

Uh oh!

fparain May 22, 2026

Choose a reason for hiding this comment

Uh oh!

fparain May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fparain commented May 29, 2026

Uh oh!

matias9927 commented Jun 1, 2026

Uh oh!

openjdk Bot commented Jun 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iklam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DanHeidinga Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

fparain Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openjdk Bot commented Jun 4, 2026

Uh oh!

Uh oh!

fparain left a comment

Choose a reason for hiding this comment

Uh oh!

matias9927 commented Jun 4, 2026

Uh oh!

openjdk Bot commented Jun 4, 2026

Uh oh!

openjdk Bot commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

matias9927 commented May 21, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented May 21, 2026 •

edited

Loading

mlbridge Bot commented May 22, 2026 •

edited

Loading