8355638: Allow -Xlog:aot to be used as an alias for -Xlog:cds when using AOT cache by iklam · Pull Request #24895 · openjdk/jdk

iklam · 2025-04-26T05:55:34Z

Specification:

When the JVM is started with any of the following options: AOTCache, AOTCacheOutput, AOTConfiguration, AOTMode:

Any -Xlog options that starts with the aot tag should also match any LogTagSets whose first tag is LogTag::_cds
When printing a LogTagSet whose first tag is LogTag::_cds, if its tags decoration is to be printed, then the first tag in the decoration should be printed as aot.

Examples:

Control case -- this is an example of an old "cds" log. The decoration should be printed as "cds" to be backwards compatible

$ java -Xshare:auto -Xlog:cds --version | grep trying
[0.003s][info][cds] trying to map /jdk3/bld/vox/images/jdk/lib/server/classes.jsa

"aot" can be used to select the "cds" log, but the log will be printed with "aot" as its decoration

$ java -XX:AOTMode=auto -Xlog:aot --version | grep trying
[0.015s][info][aot] trying to map /jdk3/bld/vox/images/jdk/lib/server/classes.jsa

When using new -XX:AOT flags, even if you specify -Xlog:cds, the output will use "aot" decoration

$ java -XX:AOTMode=auto -Xlog:cds --version | grep tryi
[0.004s][info][aot] trying to map /jdk3/bld/vox/images/jdk/lib/server/classes.jsa

When using new -XX:AOT flags, error messages should be logged with "aot" decoration even when no -Xlog flags are specified

$ java -XX:AOTMode=auto -XX:AOTCache=nofile.aot --version
[0.009s][error][aot] An error has occurred while processing the AOT cache. Run with -Xlog:aot for details.
[0.009s][error][aot] Loading AOT cache failed: nofile.aot
java 25-internal 2025-09-16
Java(TM) SE Runtime Environment (build 25-internal-adhoc.iklam.vox)
Java HotSpot(TM) 64-Bit Server VM (build 25-internal-adhoc.iklam.vox, mixed mode)

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-8355638: Allow -Xlog:aot to be used as an alias for -Xlog:cds when using AOT cache (Enhancement - P4) ⚠️ Issue is not open.

Reviewers

Johan Sjölen (@jdksjolen - Reviewer) 🔄 Re-review required (review applies to 15a87cef)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24895

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-04-26T05:56:20Z

👋 Welcome back iklam! 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-04-26T05:56:54Z

@iklam This change is no longer ready for integration - check the PR body for details.

openjdk · 2025-04-26T05:57:21Z

@iklam The following label will be automatically applied to this pull request:

hotspot-runtime

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

…w workflow; added PrintCDSLogsAsAOTLogs diagnostic flag

johan-sjolen · 2025-05-06T17:47:19Z

src/hotspot/share/logging/logSelection.cpp

 *
 */
+
+#include "cds/cds_globals.hpp"


Maybe wrap this include in INCLUDE_CDS as well?

We usually don't put includes around cds header files (there are lots of them). These header are compatible whether INCLUDE_CDS is enabled or not.

src/hotspot/share/logging/logSelectionList.cpp

mlbridge · 2025-05-06T22:33:38Z

Webrevs

vnkozlov

Looks good.

johan-sjolen

OK!

johan-sjolen · 2025-05-07T08:34:31Z

src/hotspot/share/logging/logSelection.cpp

+  if (PrintCDSLogsAsAOTLogs && _ntags > 0 && _tags[0] == LogTag::_aot && ts.tag(0) == LogTag::_cds) {
+    // Consider it a match
+    i ++;
+  }


Wrap this in INCLUDE_CDS.

vnkozlov

I found typo UL ouput in filemap.cpp: arrow_klass_pointer_bits

openjdk · 2025-05-08T01:24:00Z

@iklam this pull request can not be integrated into master 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 8355638-xlog-aot-as-alias-for-xlog-cds
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

dholmes-ora

This PR seems to be combining a few things:

establishing the "aliasing" mechanism
changing some logging statements from CDS to AOT (which already has its own JBS issue IIRC)
adding new configuration error checking

It may be cleaner to keep these distinct.

I'm also wondering whether we can have a more direct, and general-purpose, aliasing mechanism, that simply connects two tags as aliases such that enabling one enables the other and changes it's printed name? That would be less intrusive than putting this AOT specific logic (which is still an experiment/preview feature isn't it?) into the UL code.

dholmes-ora · 2025-05-08T01:18:41Z

src/hotspot/share/logging/logSelection.cpp

+#if INCLUDE_CDS
+  if (PrintCDSLogsAsAOTLogs && _ntags > 0 && _tags[0] == LogTag::_aot && ts.tag(0) == LogTag::_cds) {
+    // Consider it a match
+    i ++;


Suggested change

i ++;

i++;

iklam · 2025-05-08T02:58:50Z

This PR seems to be combining a few things:

establishing the "aliasing" mechanism

changing some logging statements from CDS to AOT (which already has its own JBS issue IIRC)

The logs that are changed are of the pattern cds+aot+link. UL cannot accept aot+aot+link:

$ java -Xlog:aot+aot --version
[0.000s][error][logging] Log selection contains duplicates of tag aot.
Invalid -Xlog option '-Xlog:aot+aot', see error log for details.

Therefore, these logs must be changed, or else they cannot be accessed using the -Xlog:aot alias. I didn't change any other cds logs.

adding new configuration error checking

Do you mean the changes in filemap.cpp? I've reverted them.

It may be cleaner to keep these distinct.

I'm also wondering whether we can have a more direct, and general-purpose, aliasing mechanism, that simply connects two tags as aliases such that enabling one enables the other and changes it's printed name? That would be less intrusive than putting this AOT specific logic (which is still an experiment/preview feature isn't it?) into the UL code.

I am not sure if we want a general-purpose, aliasing mechanism. I think it will just be confusing for both HotSpot developers and users.

I see this exercise as a one off -- a long time feature that gets a new name. Once the -Xlog:cds logs are all removed, this aliasing mechanism (a dozen or so lines of code) will be completely removed.

AOT is not an experiment/preview feature. It's a product feature in JEP 483 which was introduced in JDK 24.

stefank · 2025-05-08T16:15:07Z

This seems like a hack in the UL system. Did you consider confining this in the CDS / AOT code instead?

Something like this pseudo code:

bool aot_log_is_enabled(Level, ...) {
  return log_is_enabled(Level, aot, link) ||
         log_is_enabled(Level, cds, link);
}
void aot_log_info(...) {}
...
if (aot_log_is_enabled(Info, link) {
  ResourceMark rm;
  aot_log_info(link)("%s %s %p", class_category_name(ik), ik->external_name(), ik);
}

iklam · 2025-05-08T18:27:51Z

This seems like a hack in the UL system. Did you consider confining this in the CDS / AOT code instead?

Something like this pseudo code:
bool aot_log_is_enabled(Level, ...) {
  return log_is_enabled(Level, aot, link) ||
         log_is_enabled(Level, cds, link);
}
void aot_log_info(...) {}
...
if (aot_log_is_enabled(Info, link) {
  ResourceMark rm;
  aot_log_info(link)("%s %s %p", class_category_name(ik), ik->external_name(), ik);
}

OK I'll try that. I think most cds logs are obscure and can be changed to aot. I'll implement a few macros for the style you suggested above, and use them on the "user facing" logs -- e.g, those that report errors for trouble shooting purposes.

iklam · 2025-05-09T03:58:00Z

This seems like a hack in the UL system. Did you consider confining this in the CDS / AOT code instead?
Something like this pseudo code:
bool aot_log_is_enabled(Level, ...) {
  return log_is_enabled(Level, aot, link) ||
         log_is_enabled(Level, cds, link);
}
void aot_log_info(...) {}
...
if (aot_log_is_enabled(Info, link) {
  ResourceMark rm;
  aot_log_info(link)("%s %s %p", class_category_name(ik), ik->external_name(), ik);
}
OK I'll try that. I think most cds logs are obscure and can be changed to aot. I'll implement a few macros for the style you suggested above, and use them on the "user facing" logs -- e.g, those that report errors for trouble shooting purposes.

@stefank @jdksjolen could you check out #25136 for the new approach?

That PR has much more changes than this PR, but we need to change those eventually anyway to complete the [cds] to [aot] transition.

johan-sjolen · 2025-05-09T08:47:28Z

This seems like a hack in the UL system. Did you consider confining this in the CDS / AOT code instead?
Something like this pseudo code:
bool aot_log_is_enabled(Level, ...) {
  return log_is_enabled(Level, aot, link) ||
         log_is_enabled(Level, cds, link);
}
void aot_log_info(...) {}
...
if (aot_log_is_enabled(Info, link) {
  ResourceMark rm;
  aot_log_info(link)("%s %s %p", class_category_name(ik), ik->external_name(), ik);
}
OK I'll try that. I think most cds logs are obscure and can be changed to aot. I'll implement a few macros for the style you suggested above, and use them on the "user facing" logs -- e.g, those that report errors for trouble shooting purposes.
@stefank @jdksjolen could you check out #25136 for the new approach?

That PR has much more changes than this PR, but we need to change those eventually anyway to complete the [cds] to [aot] transition.

Personally, I was fine with a temporary hack. Now, we do have a 'proper' solution, so let's go with that.

8355638: Allow -Xlog:aot to be used as an alias for -Xlog:cds

8ed2bf5

iklam force-pushed the 8355638-xlog-aot-as-alias-for-xlog-cds branch from c32b6f7 to 8ed2bf5 Compare April 26, 2025 05:56

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Apr 26, 2025

iklam added 2 commits April 25, 2025 23:00

update

cedc5db

Simplified design/implementation: aliasing will always happen with ne…

30f2ce6

…w workflow; added PrintCDSLogsAsAOTLogs diagnostic flag

johan-sjolen reviewed May 6, 2025

View reviewed changes

iklam added 7 commits May 6, 2025 12:07

Added LogTagSet::verify_for_aot_aliases()

267fdc8

More tightening .... but this may be the wrong approach

ad6abfc

Much simplified

4a520a5

Much more simplification

ac109c3

Merge branch 'master' into 8355638-xlog-aot-as-alias-for-xlog-cds

c00b2f6

Fixed test cases

88b7ea0

clean up of existing UL logs for cds

43bdeac

openjdk bot changed the title ~~8355638: Allow -Xlog:aot to be used as an alias for -Xlog:cds~~ 8355638: Allow -Xlog:aot to be used as an alias for -Xlog:cds when using AOT cache May 6, 2025

iklam marked this pull request as ready for review May 6, 2025 22:28

openjdk bot added the rfr Pull request is ready for review label May 6, 2025

Fixed comment

15a87ce

vnkozlov approved these changes May 7, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 7, 2025

johan-sjolen approved these changes May 7, 2025

View reviewed changes

iklam added 3 commits May 7, 2025 08:34

@jdksjolen comment

2c869dc

Merge branch 'master' into 8355638-xlog-aot-as-alias-for-xlog-cds

6adfa95

cds+aot+load -> aot+load

b772b3d

openjdk bot removed the ready Pull request is ready to be integrated label May 7, 2025

vnkozlov suggested changes May 8, 2025

View reviewed changes

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label May 8, 2025

dholmes-ora reviewed May 8, 2025

View reviewed changes

Merge branch 'master' into 8355638-xlog-aot-as-alias-for-xlog-cds

8b25821

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label May 8, 2025

iklam added 3 commits May 7, 2025 19:41

@vnkozlov and @dholmes-ora comments

38dbe7e

Reverted unrelated changes in filemap.cpp

031cbef

Removed checks for error message that got removed from the PR

82e5518

iklam mentioned this pull request May 9, 2025

8356595: Convert -Xlog:cds to -Xlog:aot (step1) #25136

Closed

3 tasks

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label May 14, 2025

iklam closed this May 15, 2025

Conversation

iklam commented Apr 26, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Apr 26, 2025

Uh oh!

openjdk bot commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Apr 26, 2025

Uh oh!

johan-sjolen May 6, 2025

Choose a reason for hiding this comment

Uh oh!

iklam May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlbridge bot commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

johan-sjolen left a comment

Choose a reason for hiding this comment

Uh oh!

johan-sjolen May 7, 2025

Choose a reason for hiding this comment

Uh oh!

iklam May 7, 2025

Choose a reason for hiding this comment

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented May 8, 2025

Uh oh!

dholmes-ora left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dholmes-ora May 8, 2025

Choose a reason for hiding this comment

Uh oh!

iklam commented May 8, 2025

Uh oh!

stefank commented May 8, 2025

Uh oh!

iklam commented May 8, 2025

Uh oh!

iklam commented May 9, 2025

Uh oh!

johan-sjolen commented May 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

iklam commented Apr 26, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Apr 26, 2025 •

edited

Loading

mlbridge bot commented May 6, 2025 •

edited

Loading

dholmes-ora left a comment •

edited

Loading