8280682: Refactor AOT code source validation checks #23476

calvinccheung · 2025-02-05T22:32:58Z

This changset refactors CDS class paths and module paths validation code into a new class AOTCodeSource and related class AOTCodeSourceConfig. Code has been moved from filemap.[c|h]pp, classLoader.[c|h]pp, and classLoaderExt.[c|h]pp to aotCodeSource.[c|h]pp. CDS dependencies have been removed from classLoader.cpp. More refactoring could be done, such as removing classLoaderExt.cpp, in a future RFE.

Passed tiers 1 - 5 testing.

Progress

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

Issue

JDK-8280682: Refactor AOT code source validation checks (Enhancement - P4)

Reviewers

Ioi Lam (@iklam - Reviewer)
Ashutosh Mehra (@ashu-mehra - Committer) Review applies to 01238742
David Holmes (@dholmes-ora - Reviewer) Review applies to 9e4e33dd
Vladimir Kozlov (@vnkozlov - Reviewer) Review applies to 9e4e33dd

Contributors

Ioi Lam <[email protected]>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23476

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23476.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-02-05T22:33:30Z

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

openjdk · 2025-02-05T22:33:41Z

@calvinccheung 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:

8280682: Refactor AOT code source validation checks

Co-authored-by: Ioi Lam <[email protected]>
Reviewed-by: iklam, asmehra, dholmes, kvn

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 222 new commits pushed to the master branch:

65f79c1: 8347335: ZGC: Use limitless mark stack memory
e410af0: 8342393: Promote commutative vector IR node sharing
f755fad: 8349653: Clarify the docs for MemorySegment::reinterpret
a5c9a4d: 8349032: C2: Parse Predicate refactoring in Loop Unswitching broke fix for JDK-8290850
302bed0: 8350499: Minimal build fails with slowdebug builds
0795d11: 8350464: The flags to set the native priority for the VMThread and Java threads need a broader range
05b4812: 8350041: Skip test/jdk/java/lang/String/nativeEncoding/StringPlatformChars.java on static JDK
a891630: 8350480: RISC-V: Relax assertion about registers in C2_MacroAssembler::minmax_fp
5cbd9d1: 8349959: Test CR6740048.java passes unexpectedly missing CR6740048.xsd
25322aa: 8350258: AArch64: Client build fails after JDK-8347917
... and 212 more: https://git.openjdk.org/jdk/compare/618c5eb27b4c719afd577b690e6bcb21a45fcb0d...master

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 master branch, type /integrate in a new comment.

openjdk · 2025-02-05T22:34:14Z

@calvinccheung The following labels will be automatically applied to this pull request:

hotspot
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

calvinccheung · 2025-02-06T21:34:14Z

/contributor add @iklam

openjdk · 2025-02-06T21:35:29Z

@calvinccheung
Contributor Ioi Lam <[email protected]> successfully added.

mlbridge · 2025-02-07T20:27:48Z

Webrevs

iklam

Looks good. A few suggestions for the in-line comments.

src/hotspot/share/cds/aotCodeSource.hpp

src/hotspot/share/cds/aotCodeSource.cpp

src/hotspot/share/cds/aotCodeSource.hpp

src/hotspot/share/cds/aotCodeSource.cpp

src/hotspot/share/cds/filemap.cpp

src/hotspot/share/runtime/threads.cpp

ashu-mehra · 2025-02-18T05:21:36Z

Just one more comment, rest looks good.

dholmes-ora

This is a very large change to try and digest. Is it really just a refactor? I've made a few comments in places where it seems functionality may have been changed as well.

src/hotspot/share/cds/aotCodeSource.cpp

src/hotspot/share/cds/aotCodeSource.hpp

src/hotspot/share/runtime/threads.cpp

test/hotspot/jtreg/runtime/cds/appcds/BootClassPathMismatch.java

test/hotspot/jtreg/runtime/cds/appcds/NonExistClasspath.java

…mic archive

vnkozlov · 2025-02-20T00:35:51Z

Passing by comment. We touched it on recent Leyden meeting. The name "AOTCodeSource" is very confusing. Especially when we start caching AOT compiled code. Can we rename it to avoid confusion?

calvinccheung · 2025-02-21T06:14:18Z

Passing by comment. We touched it on recent Leyden meeting. The name "AOTCodeSource" is very confusing. Especially when we start caching AOT compiled code. Can we rename it to avoid confusion?

Per our discussions, I've renamed "AOTCodeSource" to "AOTClassLocation". I also renamed the related classes.

vnkozlov

Thank you for renaming classes.

dholmes-ora

A couple of minor suggestions, but otherwise nothing further from me.

Thanks

src/hotspot/share/cds/aotClassLocation.hpp

src/hotspot/share/cds/aotClassLocation.cpp

src/hotspot/share/classfile/classLoaderDataShared.cpp

src/hotspot/share/classfile/classLoaderDataShared.hpp

calvinccheung · 2025-02-24T19:52:50Z

@dholmes-ora, @ashu-mehra, @vnkozlov, @iklam Thanks for the reviews!

/integrate

openjdk · 2025-02-24T19:54:49Z

Going to push as commit ddb256911032cd7e6fae17c342261276066d8d25.
Since your change was applied there have been 222 commits pushed to the master branch:

65f79c1: 8347335: ZGC: Use limitless mark stack memory
e410af0: 8342393: Promote commutative vector IR node sharing
f755fad: 8349653: Clarify the docs for MemorySegment::reinterpret
a5c9a4d: 8349032: C2: Parse Predicate refactoring in Loop Unswitching broke fix for JDK-8290850
302bed0: 8350499: Minimal build fails with slowdebug builds
0795d11: 8350464: The flags to set the native priority for the VMThread and Java threads need a broader range
05b4812: 8350041: Skip test/jdk/java/lang/String/nativeEncoding/StringPlatformChars.java on static JDK
a891630: 8350480: RISC-V: Relax assertion about registers in C2_MacroAssembler::minmax_fp
5cbd9d1: 8349959: Test CR6740048.java passes unexpectedly missing CR6740048.xsd
25322aa: 8350258: AArch64: Client build fails after JDK-8347917
... and 212 more: https://git.openjdk.org/jdk/compare/618c5eb27b4c719afd577b690e6bcb21a45fcb0d...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-02-24T19:54:57Z

@calvinccheung Pushed as commit ddb2569.

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

8280682: Refactor AOT code source validation checks

ca0d9aa

openjdk bot added serviceability [email protected] hotspot [email protected] labels Feb 5, 2025

calvinccheung added 2 commits February 5, 2025 23:23

cleanup and add comments

ba9165a

trailing whitespace

816ae7e

calvinccheung marked this pull request as ready for review February 7, 2025 20:24

openjdk bot added the rfr Pull request is ready for review label Feb 7, 2025

iklam reviewed Feb 11, 2025

View reviewed changes

src/hotspot/share/cds/aotCodeSource.hpp Outdated Show resolved Hide resolved

src/hotspot/share/cds/aotCodeSource.cpp Outdated Show resolved Hide resolved

ashu-mehra reviewed Feb 13, 2025

View reviewed changes

@iklam and @ashu-mehra comment

0123874

ashu-mehra approved these changes Feb 18, 2025

View reviewed changes

ashu-mehra reviewed Feb 18, 2025

View reviewed changes

src/hotspot/share/runtime/threads.cpp Outdated Show resolved Hide resolved

dholmes-ora reviewed Feb 18, 2025

View reviewed changes

calvinccheung added 2 commits February 19, 2025 22:35

@ashu-mehra and @dholmes-ora comments

84206ed

new test case in BootClassPathMismatch.java is not applicable to dyna…

c203992

…mic archive

rename classes and add vm_exit_during_initialization call

9e4e33d

vnkozlov approved these changes Feb 21, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 21, 2025

dholmes-ora approved these changes Feb 24, 2025

View reviewed changes

@dholmes-ora comments

70f3efb

openjdk bot removed the ready Pull request is ready to be integrated label Feb 24, 2025

iklam approved these changes Feb 24, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 24, 2025

openjdk bot added the integrated Pull request has been integrated label Feb 24, 2025

openjdk bot closed this Feb 24, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 24, 2025

calvinccheung deleted the 8280682-refactor-code-source-check branch February 25, 2025 00:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8280682: Refactor AOT code source validation checks #23476

8280682: Refactor AOT code source validation checks #23476

calvinccheung commented Feb 5, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 5, 2025

openjdk bot commented Feb 5, 2025 •

edited

Loading

openjdk bot commented Feb 5, 2025

calvinccheung commented Feb 6, 2025

openjdk bot commented Feb 6, 2025

mlbridge bot commented Feb 7, 2025 •

edited

Loading

iklam left a comment

ashu-mehra commented Feb 18, 2025

dholmes-ora left a comment

vnkozlov commented Feb 20, 2025

calvinccheung commented Feb 21, 2025

vnkozlov left a comment

dholmes-ora left a comment

calvinccheung commented Feb 24, 2025

openjdk bot commented Feb 24, 2025

openjdk bot commented Feb 24, 2025

8280682: Refactor AOT code source validation checks #23476

8280682: Refactor AOT code source validation checks #23476

Conversation

calvinccheung commented Feb 5, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Feb 5, 2025

openjdk bot commented Feb 5, 2025 • edited Loading

openjdk bot commented Feb 5, 2025

calvinccheung commented Feb 6, 2025

openjdk bot commented Feb 6, 2025

mlbridge bot commented Feb 7, 2025 • edited Loading

Webrevs

iklam left a comment

Choose a reason for hiding this comment

ashu-mehra commented Feb 18, 2025

dholmes-ora left a comment

Choose a reason for hiding this comment

vnkozlov commented Feb 20, 2025

calvinccheung commented Feb 21, 2025

vnkozlov left a comment

Choose a reason for hiding this comment

dholmes-ora left a comment

Choose a reason for hiding this comment

calvinccheung commented Feb 24, 2025

openjdk bot commented Feb 24, 2025

openjdk bot commented Feb 24, 2025

calvinccheung commented Feb 5, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Feb 5, 2025 •

edited

Loading

mlbridge bot commented Feb 7, 2025 •

edited

Loading