Skip to content

8385743: [lworld] investigate and address build related comments in the Valhalla PR#2541

Open
dcubed-ojdk wants to merge 2 commits into
openjdk:lworldfrom
dcubed-ojdk:JDK-8385743
Open

8385743: [lworld] investigate and address build related comments in the Valhalla PR#2541
dcubed-ojdk wants to merge 2 commits into
openjdk:lworldfrom
dcubed-ojdk:JDK-8385743

Conversation

@dcubed-ojdk

@dcubed-ojdk dcubed-ojdk commented Jun 12, 2026

Copy link
Copy Markdown
Member

I've taken a pass at resolving most of @erikj79's build related comments in the Valhalla PR:

JDK-8317279 Standard library implementation of value classes and objects
openjdk/jdk#31123

A couple of the comments will be handled by separate issues:

  • JDK-8386451 [lworld] things to delete from lworld just before integrating JEP-401
  • JDK-8386458 [lworld] add support for allowing value classes to declare native methods

In this PR, I have reverted two of @MrSimms' heap sizes of 3200 to the main-line's 2048.
I am testing this change with a Mach5 Tier[1-8]. Mach5 Tier[123458] have completed
and there are no related test or task failures. Mach5 Tier[67] are still executing and
are currently waiting for Mach5 resources.

Update: I have added links to the comments in JDK-8317279 that discuss my analysis
of the original code, where it came from and my proposed fix; gory details as it were.



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-8385743: [lworld] investigate and address build related comments in the Valhalla PR (Bug - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2541

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

Using diff file

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

Using Webrev

Link to Webrev Comment

@dcubed-ojdk

Copy link
Copy Markdown
Member Author

Ping @erikj79 and @MrSimms - Please review when you get the chance. Thanks!

@bridgekeeper

bridgekeeper Bot commented Jun 12, 2026

Copy link
Copy Markdown

👋 Welcome back dcubed! 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.

Comment thread make/autoconf/boot-jdk.m4
JVM_HEAP_LIMIT_32="768"
# Running a 64 bit JVM allows for and requires a bigger heap
JVM_HEAP_LIMIT_64="3200"
JVM_HEAP_LIMIT_64="2048"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Use hard-coded values for java flags (one size, fits all!)
JAVA_FLAGS := -Duser.language=en -Duser.country=US
JAVA_FLAGS_BIG := -Xms64M -Xmx3200M
JAVA_FLAGS_BIG := -Xms64M -Xmx2048M

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@openjdk

openjdk Bot commented Jun 12, 2026

Copy link
Copy Markdown

@dcubed-ojdk 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:

8385743: [lworld] investigate and address build related comments in the Valhalla PR

Reviewed-by: liach

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 no new commits pushed to the lworld branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

@openjdk openjdk Bot added the rfr Pull request is ready for review label Jun 12, 2026
Comment on lines -54 to +58
JVM_EXCLUDE_FILES += templateInterpreter.cpp templateInterpreterGenerator.cpp \
bcEscapeAnalyzer.cpp ciTypeFlow.cpp macroAssembler_common.cpp
JVM_CFLAGS_FEATURES += -DZERO -DZERO_LIBARCH='"$(OPENJDK_TARGET_CPU_LEGACY_LIB)"' $(LIBFFI_CFLAGS)
JVM_EXCLUDE_FILES += templateInterpreter.cpp \
templateInterpreterGenerator.cpp bcEscapeAnalyzer.cpp ciTypeFlow.cpp \
macroAssembler_common.cpp
JVM_CFLAGS_FEATURES += -DZERO \
-DZERO_LIBARCH='"$(OPENJDK_TARGET_CPU_LEGACY_LIB)"' $(LIBFFI_CFLAGS)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Generate the value class replacements for selected java.base source files

java.base-VALUE_CLASS-REPLACEMENTS := \
JAVA_BASE_VALUE_CLASS_REPLACEMENTS := \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -42 to +49
VARHANDLE_$1_type := $$(strip $$(if $$(filter $(VARHANDLE_OBJECT_TYPES), $1), Object, $1))
VARHANDLE_$1_InputType := $$(strip $$(if $$(filter $(VARHANDLE_OBJECT_TYPES), $1), $1, $$(call Conv, $1, Type)))
VARHANDLE_$1_Type := $$(strip $$(subst NonAtomicReference, Reference, $$(subst NonAtomicFlatValue, FlatValue, $$(VARHANDLE_$1_InputType))))
VARHANDLE_$1_type := \
$$(strip $$(if $$(filter $(VARHANDLE_OBJECT_TYPES), $1), Object, $1))
VARHANDLE_$1_InputType := \
$$(strip $$(if $$(filter $(VARHANDLE_OBJECT_TYPES), $1), $1, \
$$(call Conv, $1, Type)))
VARHANDLE_$1_Type := \
$$(strip $$(subst NonAtomicReference, Reference, \
$$(subst NonAtomicFlatValue, FlatValue, $$(VARHANDLE_$1_InputType))))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -31 to +34
# valueClassPlugin.jar -- contains ValueClassPlugin (a javac Plugin that
# rewrites @AsValueClass classes to value classes
# at parse time) together with @AsValueClass and
# the META-INF service descriptor, compiled WITH
# --enable-preview and the required internal-API
# exports.
# valueClassPlugin.jar -- Contains ValueClassPlugin (a javac Plugin that
# rewrites @AsValueClass classes to value classes at parse time) together
# with @AsValueClass and the META-INF service descriptor, compiled WITH
# --enable-preview and the required internal-API exports.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -90 to +88
include MakeFileEnd.gmk No newline at end of file
include MakeFileEnd.gmk

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This puzzled me for a minute, then I remembered that the original file was missing an at the end.

HEADERS := $(TEST_LIB_SUPPORT)/test-lib_headers, \
JAR := $(TEST_LIB_SUPPORT)/test-lib.jar, \
DISABLED_WARNINGS := try deprecation rawtypes unchecked serial cast removal preview restricted varargs dangling-doc-comments, \
DISABLED_WARNINGS := preview, \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -145 to +148
# Temporarily compile preview classes into a separate directory, and then
# copy into the correct output path location.
# We cannot compile directly into the desired directory because it's the
# compiler which creates the original '<module>/<classpath>/...' hierarchy.
# Compile preview classes into a separate directory, and then copy into the
# correct output path location. We cannot compile directly into the desired
# directory because it's the compiler which creates the original
# '<module>/<classpath>/...' hierarchy.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mlbridge

mlbridge Bot commented Jun 12, 2026

Copy link
Copy Markdown

Webrevs

@liach liach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@openjdk openjdk Bot added the ready Pull request is ready to be integrated label Jun 12, 2026
@dcubed-ojdk

Copy link
Copy Markdown
Member Author

@liach - Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Pull request is ready to be integrated rfr Pull request is ready for review

Development

Successfully merging this pull request may close these issues.

2 participants