eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.27k stars 721 forks source link

Fix tests in disabled variations of ValueTypeSystemArraycopyTests #19914

Open theresa-m opened 1 month ago

theresa-m commented 1 month ago

Once https://github.com/eclipse-openj9/openj9/pull/19911 is merged ValueTypeSystemArraycopyTests will be re-enabled with the first 3 variations, all including Xint. The remaining variations have failures.

FYI @a7ehuo @hzongaro

a7ehuo commented 3 weeks ago

Just an update on this issue. The investigation is ongoing.

Found one of root causes of the failures is that null restricted arrays that are created through jdk/internal/value/ValueClass.newArrayInstance are not recognized properly in VP and got transformed as regular array in System.arraycopy transformation.

I have a potential fix which fixes 14 out of 19 ValueTypeSystemArraycopyTests failures. The other 5 failures seem related to array flattening.

theresa-m commented 3 weeks ago

I have a draft going for array flattening https://github.com/eclipse-openj9/openj9/pull/19995

I'm planning to retest and submit this after https://github.com/eclipse-openj9/openj9/pull/19911 is merged.

a7ehuo commented 3 weeks ago

@theresa-m @hangshao0 @hzongaro with change #19911, array component J9Class has one _arrayClass pointer and one _nullRestrictedArrayClass. Both _arrayClass and _nullRestrictedArrayClass share the same signature, for example, [LSomeValueClass;

In the JIT, it uses getClassFromSignature to get the class based on the signature. getClassFromSignature eventually invokes internalFindClassInModule in classsupport.c. Currently internalFindClassInModule returns the regular array class (the non null restricted array class) for signature [LSomeValueClass;. Would/should there be any change on internalFindClassInModule? If no change, once we know the signature [LSomeValueClass;, we will always only be able to get the regular array class (the non null restricted array class) from internalFindClassInModule. The reason I'm asking this question is that in the JIT, currently once we find the class from the signature, it is concluded a fixed class and a few of VP optimizations are based on it.

hangshao0 commented 3 weeks ago

Would/should there be any change on internalFindClassInModule? If no change, once we know the signature [LSomeValueClass;, we will always only be able to get the regular array class (the non null restricted array class) from internalFindClassInModule.

We could add a new option flag that can be passed to the last parameter of internalFindClassInModule() return the null restricted array.

a7ehuo commented 3 weeks ago

We could add a new option flag that can be passed to the last parameter of internalFindClassInModule() return the null restricted array.

That would help solve one problem. I think the problem for the JIT here is that when we have a class signature, we have no idea if it is for a regular nullable array class or a null restricted array class. There are assumptions in the JIT that trust the class pointer returned from getClassFromSignature to be fixed to one specific array class, which is no longer true with null restricted arrays where two array classes could be associated with the same signature

hangshao0 commented 3 weeks ago

I think the problem for the JIT here is that when we have a class signature, we have no idea if it is for a regular nullable array class or a null restricted array class. There are assumptions in the JIT that trust the class pointer returned from getClassFromSignature to be fixed to one specific array class, which is no longer true with null restricted arrays where two array classes could be associated with the same signature

@TobiAjila Any thoughts on that ?

theresa-m commented 3 weeks ago

Not sure if it helps but at one point javac (lw5 branch) was generating Class.asNullRestrictedType before each use of a null restricted array to distinguish the two types of arrays.

tajila commented 3 weeks ago

I think the problem for the JIT here is that when we have a class signature, we have no idea if it is for a regular nullable array class or a null restricted array class. There are assumptions in the JIT that trust the class pointer returned from getClassFromSignature to be fixed to one specific array class, which is no longer true with null restricted arrays where two array classes could be associated with the same signature

The Null-restricted array doubles down on erasure in that from the perspective of the classfile Foo[] and Foo![] are both [LFoo. This means anywhere [LFoo is encountered you need to be prepared to deal with both nullable and null-restricted unless you can prove it goes one way or the other. In the interpreter we rely on runtime checks for this.

The JIT will need to do something similar. Either prove that it must be Foo![] or Foo[] or be prepared for both. You can prove that its not Foo[] if you know the array slot was populated with Class.asNullRestrictedType or something similar. There are other techniques as well.

In either case looking for the array with [LFoo is the right approach because you can get both array classes from each other.

We could add a new option flag that can be passed to the last parameter of internalFindClassInModule() return the null restricted array.

So Im not really in favour of this, because you need to know what you are looking for, and if you know you can always find it with the regular array class.

hangshao0 commented 3 weeks ago

and if you know you can always find it with the regular array class.

The current code of internalFindClassInModule() only looks for the nullable array. It is possible that only null restricted array J9Class has been created, nullable array J9Class is NULL.

tajila commented 3 weeks ago

The current code of internalFindClassInModule() only looks for the nullable array. It is possible that only null restricted array J9Class has been created, nullable array J9Class is NULL.

With normal usage partterns, given that javac will lookup the NR class from the nullable one then we should be guaranteed that the nullable one is loaded. But I guess someone can handwrite bytecodes that break this assumption.

What we could do is always load the nullable array class as a pre-requisite for the NR array that way if the JIT looks up [LFoo and and either Foo![] or Foo[] is created there is always an answer.