Skip to content

Rename Preload attribute to LoadableDescriptors - #20325

Merged
hangshao0 merged 3 commits into
eclipse-openj9:masterfrom
theresa-m:fix_19611
Oct 22, 2024
Merged

Rename Preload attribute to LoadableDescriptors#20325
hangshao0 merged 3 commits into
eclipse-openj9:masterfrom
theresa-m:fix_19611

Conversation

@theresa-m

@theresa-m theresa-m commented Oct 9, 2024

Copy link
Copy Markdown
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

keithc-ca commented Oct 9, 2024

Copy link
Copy Markdown
Contributor

Please address the line endings check failure.

keithc-ca
keithc-ca requested changes Oct 9, 2024
View reviewed changes
Comment thread debugtools/DDR_VM/src/com/ibm/j9ddr/CompatibilityConstants29.dat
Comment thread debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/j9/OptInfo.java
Comment thread runtime/cfdumper/main.c Outdated
Comment thread runtime/nls/cfre/cfrerr.nls Outdated
Comment thread runtime/oti/util_api.h Outdated
Comment thread runtime/vm/createramclass.cpp Outdated
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaUtils.java Outdated
@theresa-m
ValhallaAttributeTests Preload to LoadableDescriptors
6d58520 
Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>
@theresa-m
Vm update from Preload to LoadableDescriptor
d7ee5c0 
Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>
@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
keithc-ca
keithc-ca requested changes Oct 15, 2024
View reviewed changes
Comment thread runtime/vm/createramclass.cpp Outdated
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated
Comment thread test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java Outdated
@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

keithc-ca commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

Jenkins test sanity aarch64_linux,aarch64_linux_vt_standard jdknext

@keithc-ca

keithc-ca commented Oct 17, 2024

Copy link
Copy Markdown
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

keithc-ca commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

I created #20377 which should unblock this.

@pshipton

pshipton commented Oct 17, 2024

Copy link
Copy Markdown
Member

Jenkins test sanity aarch64_linux_vt_standard jdknext

@keithc-ca

keithc-ca commented Oct 17, 2024

Copy link
Copy Markdown
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

theresa-m commented Oct 18, 2024

Copy link
Copy Markdown
Contributor Author

Those failures are related to #20372

@hangshao0

hangshao0 commented Oct 18, 2024

Copy link
Copy Markdown
Contributor

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

@keithc-ca

keithc-ca commented Oct 18, 2024

Copy link
Copy Markdown
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
Comment thread 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.

@hangshao0 hangshao0 Oct 21, 2024

Copy link
Copy Markdown
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 <Theresa.T.Mammarella@ibm.com>
@theresa-m
theresa-m requested a review from hangshao0 October 21, 2024 19:35
@hangshao0

hangshao0 commented Oct 22, 2024

Copy link
Copy Markdown
Contributor

Jenkins test sanity.functional,extended alinuxval jdknext

@hangshao0
hangshao0 merged commit bd36b1d into eclipse-openj9:master Oct 22, 2024
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