Open cjjdespres opened 2 months ago
Some more details:
From the JIT dump, the method in question has the byte codes:
0, JBaload0
1, JBiconst0
2, JBinvokestatic 15
5, JBreturn0
and apparently zero instructions. Also, the headers of currentRomClass
and compileRomClass
appear to be identical, except for:
currentRomClass:
romSize = 3008
intermediateClassDataLength = 3008
compileRomClass:
romSize = 3176
intermediateClassDataLength = 3176
The addresses of the ROM classes are of course different. The name of both ROM classes is SymbolicDescTest
.
In JITServerHelpers::packROMClass
, we strip out the intermediateClassData
completely from a ROM class when packing the ROM class (to send to the server, or to calculate its hash) because it is "not used by the JIT", according to a comment in that method. The romSize
and intermediate class data fields are also fixed up. The ROM classes being entirely intermediate class data is consistent with the "compiled" version of the method that caused the assert having no instructions.
There is the following in the java core:
3CLTEXTCLASS SymbolicDescTest(0x0000000000559900)
3CLTEXTCLASS SymbolicDescTest(0x000000000059E100)
and the addresses of the ROM classes associated to these RAM classes match those of compileRomClass
and currentRomClass
, respectively.
So, the deserializer happened to resolve a particular class record to that first J9Class *
. The SVM looked up that class's J9ROMClass *
, and that became compileRomClass
. The SVM also used different records to look up what the currentMethod
ought to be, and found that its defining class was that second SymbolicDescTest
instance, and so had a different underlying ROM class.
@dsouzai Maybe this particular relocation should succeed? We retrieve the currentMethod
here:
and the assert that failed is just down from there. The deserializer will guarantee that the compileRomClass
has equal name, packed length, and packed hash to the ROM class at compile time, and it should therefore always be equal to currentRomClass
in those properties (I believe). So it seems plausible to me that if we are relocating a method that was deserialized, we should be able to skip this pointer equality check. If not, we can just turn off the assert in that circumstance.
In a local AOT compilation, the reason for this check is that although compileRomClass
and currentRomClass
are acquired using different means, they were the same value in the compile run:
https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/codegen/J9AheadOfTimeCompile.cpp#L545 https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/codegen/J9AheadOfTimeCompile.cpp#L551 https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/codegen/J9AheadOfTimeCompile.cpp#L572 https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/codegen/J9AheadOfTimeCompile.cpp#L579
Therefore, on load when we get use that info to get the rom class: https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/runtime/RelocationRecord.cpp#L2790 https://github.com/eclipse-openj9/openj9/blob/84e2c9d1715c56f45e3a9bebbbbf3b88673b06c6/runtime/compiler/runtime/RelocationRecord.cpp#L2819-L2820
they should get the same rom class.
Now in the case of JITServer+AOTCache, during relocation, is it possible for there to be two different ROMClass pointers that are actually the same ROMClass on the client?
Now in the case of JITServer+AOTCache, during relocation, is it possible for there to be two different ROMClass pointers that are actually the same ROMClass on the client?
Yes, this can happen. The checks during deserialization will guarantee that compileRomClass
has the same size and hash as what the ROM class had at compile time after packing, which inlines all UTF8 strings and discards intermediate class data and debug info. So compileRomClass
should be equivalent to currentRomClass
from a JIT perspective under SVM, but may not be equal to it.
This particular assert was triggered because there were two ROM classes in the SCC that were equal in content, except that they had slightly different intermediate class data. The class the deserializer cached for this offset happened to be the "wrong" one, but it was still equivalent to the "right" one, the one that the relo runtime was expecting.
This particular assert was triggered because there were two ROM classes in the SCC that were equal in content, except that they had slightly different intermediate class data. The class the deserializer cached for this offset happened to be the "wrong" one, but it was still equivalent to the "right" one, the one that the relo runtime was expecting.
When you say two ROM classes in the SCC, do you mean in the client's SCC? If so, wouldn't they actually be two different classes?
Two distinct ROM classes in the client's SCC. They just happen to have equal packed hash. Since JITServerHelpers::packROMClass()
seems to be of the opinion that they're equivalent from the perspective of the JIT compiler, I thought that that might be good enough to pass validation here.
The answer currently in the code is no, that's not good enough to pass validation. It's just that the current AOT cache hashing scheme and serialization record structure isn't strong enough to guarantee that after deserialization, when the ROM class offsets have been adjusted in the method's relocation records, that compileRomClass
will be equal (in the sense of pointer equality) to currentRomClass
, only that they will have equal packed hashes. That happens to conflict with this SVM assert.
Two distinct ROM classes in the client's SCC. They just happen to have equal packed hash. Since JITServerHelpers::packROMClass() seems to be of the opinion that they're equivalent from the perspective of the JIT compiler, I thought that that might be good enough to pass validation here.
My initial feeling is that if they are distinct classes in the client's SCC, then JITServerHelpers::packROMClass()
should also be able to distinguish them. However, I do agree that if JITServerHelpers::packROMClass()
thinks they're equivalent, then it should be consistent on the load run as well. That said, I think the comparison between compileRomClass
and currentRomClass
should still exist, it's just that if they aren't equal, then under JITServer+AOTCache, we do an additional check to see if their packed hash are equal.
@hangshao0, when the SCC has two ROMClasses that are basically the same except for intermediate class data fields, what does that mean from a java class pov? How can there be two different ROMClasses with the same name? Is it that they come from different places on disk?
when the SCC has two ROMClasses that are basically the same except for intermediate class data fields, what does that mean from a java class pov?
If they are exactly the same except the intermediate class data, these 2 classes will be treated as the same class in the JVM code.
How can there be two different ROMClasses with the same name? Is it that they come from different places on disk?
Yes, they could come from different places on the disk. Even the same jar on the disk can be re-compiled with newer version of classes, so there could be 2 versions of class under the same name.
@cjjdespres I guess given what Hang said, what JITServerHelpers::packROMClass()
currently does is fine. The relo code would just need to be adjusted to check the packed hash if the two pointers don't match.
I observed this at a client JVM when running
jdk_lang_0
with a manually-started JITServer on the side, usingEXTRA_OPTIONS
to enable the JITServer AOT cache on the client. This is a non-fatal SVM assert. Console log:This was a JITServer AOT cache method received at the client. This shouldn't be a functional issue - when running with non-fatal SVM asserts (the default) we simply abort the relocation if this happens. It should still be addressed, of course.