Remove default value initialization #23089

AditiS11 · 2025-12-10T11:35:20Z

Related: #22642
These changes aim to align the code with JEP 401 strict field initialization. It includes:
-Remove the J9ClassContainsUnflattenedFlattenables flag.
-Remove default value initialization.
-Re-enable disabled tests.

Signed-off-by: Aditi Srinivas M Aditi.Srini@ibm.com

theresa-m · 2025-12-11T20:04:29Z

runtime/compiler/codegen/J9AheadOfTimeCompile.cpp

            return "TR_VirtualRamMethodConst";
         case TR_ClassAddress:
            return "TR_ClassAddress";
-         case TR_StaticDefaultValueInstance:


I see some uses of TR_StaticDefaultValueInstance in OMR as well that should be removed. You can do this in a pull request after this one has been merged to avoid compilation issues.

theresa-m · 2025-12-11T20:07:40Z

runtime/compiler/compile/J9SymbolReferenceTable.hpp

   TR::SymbolReference * findOrFabricateFlattenedArrayElementFieldShadowSymbol(TR_OpaqueClassBlock *arrayComponentClass, TR::DataType type, int32_t fieldOffset, bool isPrivate, const char *fieldName, const char *fieldSignature);

   /** \brief
    *     Returns a symbol reference for default value instance of value class.


The comment for this method can also be removed.

theresa-m · 2025-12-15T22:04:31Z

runtime/compiler/compile/J9SymbolReferenceTable.cpp

-TR::SymbolReference *
-J9::SymbolReferenceTable::findOrCreateDefaultValueSymbolRef(void *defaultValueSlotAddress, int32_t cpIndex)
-   {
-   ListIterator<TR::SymbolReference> i(&_defaultValueAddressSlotSymbolRefs);


Remove other references to _defaultValueAddressSlotSymbolRefs

theresa-m · 2025-12-15T22:06:14Z

runtime/compiler/control/JITClientCompilationThread.cpp

-         {
-         auto recv = client->getRecvData<TR_OpaqueClassBlock *>();
-         auto clazz = std::get<0>(recv);
-         client->write(response, TR::Compiler->cls.getDefaultValueSlotAddress(comp, clazz));


Remove getDefaultValueSlotAddress from J9ClassEnv.hpp.

theresa-m · 2025-12-15T22:11:05Z

runtime/compiler/env/VMJ9.cpp

   if (clazz->initializeStatus != 1)
      return false;

   // Cannot inline the allocation if the class is an interface, abstract,


Please update this comment.

theresa-m · 2025-12-16T15:27:50Z

runtime/vm/BytecodeInterpreter.hpp

-		if (skipUnflattenedFlattenableCheck || J9_ARE_NO_BITS_SET(clazz->classFlags, J9ClassContainsUnflattenedFlattenables))
-#endif /* J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES */
-		{
-			instance = _objectAllocate.inlineAllocateObject(_currentThread, clazz, initializeSlots, memoryBarrier);


This line is still needed.

theresa-m · 2025-12-16T15:28:48Z

runtime/vm/BytecodeInterpreter.hpp

-		if (J9_ARE_NO_BITS_SET(arrayClass->classFlags, J9ClassContainsUnflattenedFlattenables))
-#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
-		{
-			instance = VM_VMHelpers::inlineAllocateIndexableObject(_currentThread, &_objectAllocate, arrayClass, size, initializeSlots, memoryBarrier, sizeCheck);


This line is still needed.

theresa-m · 2025-12-17T15:55:26Z

runtime/vm/BytecodeInterpreter.hpp

-				if (J9_ARE_NO_BITS_SET(resolvedClass->classFlags, J9ClassContainsUnflattenedFlattenables))
-#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
-				{
-					instance = _objectAllocate.inlineAllocateObject(_currentThread, resolvedClass);


This line can stay.

theresa-m · 2025-12-17T15:55:36Z

runtime/vm/BytecodeInterpreter.hpp

-				if (J9_ARE_NO_BITS_SET(arrayClass->classFlags, J9ClassContainsUnflattenedFlattenables))
-#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
-				{
-					instance = VM_VMHelpers::inlineAllocateIndexableObject(_currentThread, &_objectAllocate, arrayClass, (U_32) size);


This line can stay.

theresa-m · 2025-12-17T15:59:28Z

runtime/vm/BytecodeInterpreter.hpp

 		if (NULL != obj && NULL != clz) {
 			J9Class *clzJ9Class = J9VM_J9CLASS_FROM_HEAPCLASS(_currentThread, clz);

-			if (J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(clzJ9Class)) {


Please leave this and line 4546 unchanged for now. getFlattenedFieldAtOffset only handles flattened cases and with the introduction of null-restricted types we can't tell if its flattened just from the J9Class.

I made this change as several ValueTypeUnsafe tests (eg: testGetValuesOfObject) were failing with a NullPointerException as Unsafe.getValue currently returns null for non-primitive value types.
Broadening the check to J9_IS_J9CLASS_VALUETYPE allows the tests to pass.

For eg:

FAILED: testGetValuesOfObject java.lang.NullPointerException at org.openj9.test.lworld.ValueTypeUnsafeTests.testGetValuesOfObject(ValueTypeUnsafeTests.java:306) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:571) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124) at org.testng.internal.Invoker.invokeMethod(Invoker.java:580) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:716) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:988) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109) at org.testng.TestRunner.privateRun(TestRunner.java:648) at org.testng.TestRunner.run(TestRunner.java:505) at org.testng.SuiteRunner.runTest(SuiteRunner.java:455) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:450) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:415) at org.testng.SuiteRunner.run(SuiteRunner.java:364) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:84) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1208) at org.testng.TestNG.runSuitesLocally(TestNG.java:1137) at org.testng.TestNG.runSuites(TestNG.java:1049) at org.testng.TestNG.run(TestNG.java:1017) at org.testng.TestNG.privateMain(TestNG.java:1354) at org.testng.TestNG.main(TestNG.java:1323) ```

I see, this was passing before because J9ClassAllowsInitialDefaultValue and J9ClassIsPrimitiveValueType flags shared the same value.

VM_ValueTypeHelpers::getFlattenedFieldAtOffset says it should only be used with flattened fields but it clearly hasn't been used that way for a while. I don't see why it needs to be used for only flattened fields though is there something I'm missing @hangshao0 ?

Looks like getValue is replaced now upstream https://bugs.openjdk.org/browse/JDK-8373374

I don't see why it needs to be used for only flattened fields though is there something I'm missing @hangshao0 ?

getFlattenedFieldAtOffset() calls copyObjectFields() to copy the data. It copies from the given offset for X bytes, where X is the size of the field type when it is flattened. So it can only been used on flattened fields.

openj9/runtime/gc_base/ObjectAccessBarrier.cpp

Line 1606 in bf01348

UDATA limit = J9CLASS_UNPADDED_INSTANCE_SIZE(objectClass);

#define J9CLASS_UNPADDED_INSTANCE_SIZE(clazz) J9_VALUETYPE_FLATTENED_SIZE(clazz)

Okay. I think for now the getValue and putValue code should remain the same and the tests that use it should be disabled. Once the extensions repo has been updated to include getFlatValue/putFlatValue instead of getValue/putValue the api and tests can be updated.

Do you mind creating an issue for this @AditiS11 ?

Sure will do it.

a7ehuo

Thank you for the change @AditiS11! I took a look at files under runtime/compiler.

Here are my comments. Meanwhile, the commit message should follow the format stated in OpenJ9 CONTRIBUTING.md and it is especially missing Signed-off-by: Full Name <email>.

openj9/CONTRIBUTING.md

Lines 116 to 136 in ec64be7

    
           When a commit has related issues or commits, explain the relation in the message 
        
           body. When appropriate, use the keywords described in the following help article 
        
           to automatically close issues. 
        
           https://help.github.com/articles/closing-issues-using-keywords/ 
        
           For example: 
        
           ``` 
        
           Correct race in frobnicator 
        
           This patch eliminates the race condition in issue #1234. 
        
           Fixes: #1234 
        
           ``` 
        
           Sign off on your commit in the footer. By doing this, you assert original 
        
           authorship of the commit and that you are permitted to contribute it. This can 
        
           be automatically added to your commit by passing `-s` to `git commit`, or by 
        
           manually adding the following line to the footer of the commit. 
        
           ``` 
        
           Signed-off-by: Full Name <email>

a7ehuo · 2025-12-17T21:55:23Z

runtime/compiler/optimizer/EscapeAnalysis.cpp

         if (candidate->_kind == TR::New)
            {
            TR_OpaqueClassBlock *clazz = (TR_OpaqueClassBlock *)candidate->_node->getFirstChild()->getSymbol()->getStaticSymbol()->getStaticAddress();
-


I believe this whole if branch should be removed.

// Primitive value type fields of objects created with a NEW bytecode must be initialized // with their default values. EA is not yet set up to perform such iniitialization // if the value type's own fields have not been inlined into the class that // has a field of that type, so remove the candidate from consideration. if (candidate->_kind == TR::New) { TR_OpaqueClassBlock *clazz = (TR_OpaqueClassBlock *)candidate->_node->getFirstChild()->getSymbol()->getStaticSymbol()->getStaticAddress(); if (!TR::Compiler->cls.isZeroInitializable(clazz)) { logprintf(trace(), log, " Fail [%p] because the candidate is not zero initializable (that is, it has a field of a primitive value type whose fields have not been inlined into this candidate's class)\n", candidate->_node); rememoize(candidate); _candidates.remove(candidate); continue; } }

a7ehuo · 2025-12-17T22:16:03Z

runtime/compiler/control/JITServerCompilationThread.cpp

      if(!(std::get<0>(classInfoTuple).empty()))
         {
-         romClass = JITServerHelpers::romClassFromString(std::get<0>(classInfoTuple), std::get<25>(classInfoTuple), clientSession->persistentMemory());
+         romClass = JITServerHelpers::romClassFromString(std::get<0>(classInfoTuple), std::get<24>(classInfoTuple), clientSession->persistentMemory());


These changes in JITServer would require to update MINOR_NUMBER in CommunicationStream.hpp

a7ehuo · 2025-12-17T22:38:24Z

runtime/compiler/env/J9ClassEnv.hpp

-    *    `false` otherwise (that is, if the class has value type fields whose fields have not
-    *    been inlined)
-    */
-   bool isZeroInitializable(TR_OpaqueClassBlock *clazz);


getDefaultValueSlotAddress needs to be removed in this file too

a7ehuo · 2025-12-17T22:42:26Z

runtime/compiler/codegen/J9AheadOfTimeCompile.cpp

            return "TR_VirtualRamMethodConst";
         case TR_ClassAddress:
            return "TR_ClassAddress";
-         case TR_StaticDefaultValueInstance:


The reference to isStaticDefaultValueInstance() in J9SymbolReference.cpp can be removed

a7ehuo · 2025-12-17T22:52:22Z

runtime/compiler/env/VMJ9.cpp

-   if ((clazz->romClass->modifiers & (J9AccAbstract | J9AccInterface))
-       || (clazz->classFlags & J9ClassContainsUnflattenedFlattenables))


extra bracket

if (clazz->romClass->modifiers & (J9AccAbstract | J9AccInterface))

The comments above the code should be updated to reflect the removed code

// Can not inline the allocation if the class is an interface or abstract

a7ehuo · 2025-12-17T22:56:05Z

runtime/compiler/env/VMJ9Server.cpp

-         {
-         uintptr_t classFlags = 0;
-         JITServerHelpers::getAndCacheRAMClassInfo((J9Class*)clazz, _compInfoPT->getClientData(), stream, JITServerHelpers::CLASSINFO_CLASS_FLAGS, (void*)&classFlags);
-         return (classFlags & J9ClassContainsUnflattenedFlattenables) ? false : true;


Maybe something as below after removing the check on J9ClassContainsUnflattenedFlattenables:

if (isClassInitialized) return ((modifiers & (J9AccAbstract | J9AccInterface )) ? false : true);

theresa-m · 2025-12-24T15:39:05Z

runtime/gc_structs/ValueTypesIterator.hpp

-			break;
-		}
-
 		return NULL;


This whole iterator can be removed since it will never return a value.

theresa-m · 2025-12-24T15:51:32Z

runtime/vm/BytecodeInterpreter.hpp

+	allocateObject(REGISTER_ARGS_LIST, J9Class *clazz, bool initializeSlots = true, bool memoryBarrier = true)
 	{
 		j9object_t instance = NULL;
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)


This line and line 1140 can be removed.

theresa-m · 2025-12-24T15:54:10Z

runtime/vm/BytecodeInterpreter.hpp

 	allocateIndexableObject(REGISTER_ARGS_LIST, J9Class *arrayClass, U_32 size, bool initializeSlots = true, bool memoryBarrier = true, bool sizeCheck = true)
 	{
 		j9object_t instance = NULL;
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)


This line and 1169 can be removed.

theresa-m · 2025-12-24T15:56:09Z

runtime/vm/BytecodeInterpreter.hpp

 		if ((NULL != resolvedClass) && J9ROMCLASS_ALLOCATES_VIA_NEW(resolvedClass->romClass)) {
 			if (!VM_VMHelpers::classRequiresInitialization(_currentThread, resolvedClass)) {
 				j9object_t instance = NULL;
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)


Also here and line 8918.

Related: eclipse-openj9#22642 Remove the J9ClassContainsUnflattenedFlattenables flag Remove default value initialization. Re-enable disabled tests Signed-off-by: Aditi Srinivas M <Aditi.Srini@ibm.com>

AditiS11 requested a review from dsouzai as a code owner December 10, 2025 11:35

AditiS11 marked this pull request as draft December 10, 2025 17:34

AditiS11 marked this pull request as ready for review December 11, 2025 06:14

hangshao0 added comp:vm project:valhalla Used to track Project Valhalla related work comp:test labels Dec 11, 2025

hangshao0 requested review from theresa-m and removed request for dsouzai December 11, 2025 15:24

AditiS11 force-pushed the implicit_3 branch from 453ac24 to 6acf1f0 Compare December 11, 2025 15:40

hzongaro added comp:jit comp:jit:aot comp:jitserver Artifacts related to JIT-as-a-Service project labels Dec 12, 2025

hzongaro requested review from a7ehuo and hzongaro December 12, 2025 18:03

theresa-m mentioned this pull request Dec 15, 2025

Valhalla implementation required for latest OpenJDK/Valhalla updates #22642

Open

theresa-m suggested changes Dec 17, 2025

View reviewed changes

a7ehuo reviewed Dec 18, 2025

View reviewed changes

AditiS11 force-pushed the implicit_3 branch 2 times, most recently from 5a1c307 to 39c0cd0 Compare December 23, 2025 11:43

theresa-m suggested changes Dec 24, 2025

View reviewed changes

Remove default value initialization

cf1c614

Related: eclipse-openj9#22642 Remove the J9ClassContainsUnflattenedFlattenables flag Remove default value initialization. Re-enable disabled tests Signed-off-by: Aditi Srinivas M <Aditi.Srini@ibm.com>

AditiS11 force-pushed the implicit_3 branch from 39c0cd0 to cf1c614 Compare December 26, 2025 08:20

	When a commit has related issues or commits, explain the relation in the message
	body. When appropriate, use the keywords described in the following help article
	to automatically close issues.
	https://help.github.com/articles/closing-issues-using-keywords/
	For example:

	```
	Correct race in frobnicator

	This patch eliminates the race condition in issue #1234.

	Fixes: #1234
	```

	Sign off on your commit in the footer. By doing this, you assert original
	authorship of the commit and that you are permitted to contribute it. This can
	be automatically added to your commit by passing `-s` to `git commit`, or by
	manually adding the following line to the footer of the commit.

	```
	Signed-off-by: Full Name <email>

		if ((clazz->romClass->modifiers & (J9AccAbstract \| J9AccInterface))
		\|\| (clazz->classFlags & J9ClassContainsUnflattenedFlattenables))

Remove default value initialization #23089

Are you sure you want to change the base?

Remove default value initialization #23089

Conversation

AditiS11 commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theresa-m Dec 11, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AditiS11 Dec 23, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7ehuo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theresa-m Dec 24, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AditiS11 commented Dec 10, 2025 •

edited

Loading

theresa-m Dec 11, 2025 •

edited

Loading

AditiS11 Dec 23, 2025 •

edited

Loading

theresa-m Dec 24, 2025 •

edited

Loading