Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename Preload attribute to LoadableDescriptors #20325

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Rename Preload attribute to LoadableDescriptors #20325

merged 3 commits into from
Oct 22, 2024

Conversation

theresa-m
Copy link
Contributor

Fixes: #19611

I tried to split commits as follows for easier reviewing:

  1. Test changes
  2. VM logic changes
  3. VM renaming and formatting

@theresa-m theresa-m requested a review from hangshao0 October 9, 2024 19:52
@theresa-m theresa-m added project:valhalla Used to track Project Valhalla related work comp:vm labels Oct 9, 2024
@keithc-ca
Copy link
Contributor

Please address the line endings check failure.

keithc-ca
keithc-ca requested changes Oct 9, 2024
View reviewed changes
debugtools/DDR_VM/src/com/ibm/j9ddr/CompatibilityConstants29.dat Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/j9/OptInfo.java Show resolved Hide resolved
runtime/cfdumper/main.c Outdated Show resolved Hide resolved
runtime/nls/cfre/cfrerr.nls Outdated Show resolved Hide resolved
runtime/oti/util_api.h Outdated Show resolved Hide resolved
runtime/vm/createramclass.cpp Outdated Show resolved Hide resolved
test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated Show resolved Hide resolved
test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated Show resolved Hide resolved
test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated Show resolved Hide resolved
test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaUtils.java Outdated Show resolved Hide resolved
@theresa-m theresa-m force-pushed the fix_19611 branch 2 times, most recently from 5560675 to ff9bbc4 Compare October 15, 2024 15:48
@theresa-m theresa-m requested a review from keithc-ca October 15, 2024 15:48
@theresa-m theresa-m force-pushed the fix_19611 branch 2 times, most recently from bd3ebdb to 7715fde Compare October 16, 2024 12:48
@theresa-m theresa-m requested a review from keithc-ca October 16, 2024 12:48
@theresa-m theresa-m force-pushed the fix_19611 branch 2 times, most recently from 9f9862d to 541003f Compare October 17, 2024 15:39
@keithc-ca
Copy link
Contributor

Jenkins test sanity aarch64_linux,aarch64_linux_vt_standard jdknext

@keithc-ca
Copy link
Contributor

Updates are required to https://github.com/ibmruntimes/openj9-openjdk-jdk.valuetypes before that repository will compile with the current content of https://github.com/eclipse-openj9/openj9:

[2024-10-17T16:51:51.110Z] /home/jenkins/workspace/Build_JDKnext_aarch64_linux_vt_standard_Personal/build/linux-aarch64-server-release/support/j9jcl/jdk.management/share/classes/com/ibm/lang/management/internal/PlatformMBeanProvider.java:40: error: cannot find symbol
[2024-10-17T16:51:51.110Z] import com.sun.management.internal.VirtualThreadSchedulerImpls;
[2024-10-17T16:51:51.110Z]                                   ^
[2024-10-17T16:51:51.110Z]   symbol:   class VirtualThreadSchedulerImpls
[2024-10-17T16:51:51.110Z]   location: package com.sun.management.internal
[2024-10-17T16:51:51.110Z] /home/jenkins/workspace/Build_JDKnext_aarch64_linux_vt_standard_Personal/build/linux-aarch64-server-release/support/j9jcl/jdk.management/share/classes/com/ibm/lang/management/internal/PlatformMBeanProvider.java:41: error: package jdk.management does not exist
[2024-10-17T16:51:51.110Z] import jdk.management.VirtualThreadSchedulerMXBean;
[2024-10-17T16:51:51.110Z]                      ^
[2024-10-17T16:51:51.110Z] /home/jenkins/workspace/Build_JDKnext_aarch64_linux_vt_standard_Personal/build/linux-aarch64-server-release/support/j9jcl/jdk.management/share/classes/com/ibm/lang/management/internal/PlatformMBeanProvider.java:114: error: cannot find symbol
[2024-10-17T16:51:51.110Z] 			.addInterface(VirtualThreadSchedulerMXBean.class)
[2024-10-17T16:51:51.110Z] 			              ^
[2024-10-17T16:51:51.110Z]   symbol:   class VirtualThreadSchedulerMXBean
[2024-10-17T16:51:51.110Z]   location: class PlatformMBeanProvider
[2024-10-17T16:51:51.110Z] /home/jenkins/workspace/Build_JDKnext_aarch64_linux_vt_standard_Personal/build/linux-aarch64-server-release/support/j9jcl/jdk.management/share/classes/com/ibm/lang/management/internal/PlatformMBeanProvider.java:113: error: cannot find symbol
[2024-10-17T16:51:51.110Z] 		ComponentBuilder.create("jdk.management:type=VirtualThreadScheduler", VirtualThreadSchedulerImpls.create()) //$NON-NLS-1$
[2024-10-17T16:51:51.110Z] 		                                                                      ^
[2024-10-17T16:51:51.110Z]   symbol:   variable VirtualThreadSchedulerImpls
[2024-10-17T16:51:51.110Z]   location: class PlatformMBeanProvider
[2024-10-17T16:51:51.110Z] 4 errors

@keithc-ca keithc-ca mentioned this pull request Oct 17, 2024
Merged
@keithc-ca
Copy link
Contributor

I created #20377 which should unblock this.

@pshipton
Copy link
Member

Jenkins test sanity aarch64_linux_vt_standard jdknext

@keithc-ca
Copy link
Contributor

There are many failures like this:

FAILED CONFIGURATION: @BeforeMethod setUp
java.lang.InternalError: java.lang.IllegalAccessException: static final field has no write access: jdk.internal.misc.Unsafe.theUnsafe/jdk.internal.misc.Unsafe/putStatic, from class java.lang.Object (module java.base)
	at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:207)
	at java.base/jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:144)
	at java.base/java.lang.reflect.Field.acquireOverrideFieldAccessor(Field.java:1212)
	at java.base/java.lang.reflect.Field.getOverrideFieldAccessor(Field.java:1181)
	at java.base/java.lang.reflect.Field.get(Field.java:454)
	at org.openj9.test.unsafe.UnsafeTestBase.getUnsafeInstance(UnsafeTestBase.java:120)
	at org.openj9.test.unsafe.UnsafeTestBase.setUp(UnsafeTestBase.java:562)
	[...]
Caused by: java.lang.IllegalAccessException: static final field has no write access: jdk.internal.misc.Unsafe.theUnsafe/jdk.internal.misc.Unsafe/putStatic, from class java.lang.Object (module java.base)
	at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:913)
	at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3608)
	at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3599)
	at java.base/java.lang.invoke.MethodHandleImpl$1.unreflectField(MethodHandleImpl.java:1621)
	at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:185)
	... 31 more

@theresa-m
Copy link
Contributor Author

Those failures are related to #20372

@hangshao0
Copy link
Contributor

Do you have more review comments on this ? @keithc-ca

@keithc-ca
Copy link
Contributor

Do you have more review comments on this?

No more comments other than to say I'd be happier if we didn't have to ignore test failures, but I'll defer to you, @hangshao0.

hangshao0
hangshao0 reviewed Oct 21, 2024
View reviewed changes
runtime/nls/cfre/cfrerr.nls
# LoadableDescriptors is not translatable
J9NLS_CFR_ERR_MULTIPLE_LOADABLEDESCRIPTORS_ATTRIBUTES=Multiple LoadableDescriptors attributes
# START NON-TRANSLATABLE
J9NLS_CFR_ERR_MULTIPLE_LOADABLEDESCRIPTORS_ATTRIBUTES.explanation=Please consult the Java Virtual Machine Specification for a detailed explanation.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could mention there should be only one LoadableDescriptors attribute either in the NLS message or in the explanation so that it is more obvious to the users what is wrong.

@theresa-m
Rename Preload to LoadableDescriptors
25469ba 
Signed-off-by: Theresa Mammarella <[email protected]>
@theresa-m theresa-m requested a review from hangshao0 October 21, 2024 19:35
@hangshao0
Copy link
Contributor

Jenkins test sanity.functional,extended alinuxval jdknext

@hangshao0 hangshao0 merged commit bd36b1d into eclipse-openj9:master Oct 22, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keithc-ca keithc-ca keithc-ca approved these changes

@hangshao0 hangshao0 hangshao0 approved these changes

Assignees

@hangshao0 hangshao0

Labels
comp:vm project:valhalla Used to track Project Valhalla related work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The Preload attribute is renamed to LoadableDescriptors in Valhalla
4 participants
@theresa-m @keithc-ca @pshipton @hangshao0