TR_ResolvedJ9JITServerMethod::staticsAreSame needs to be updated to be compatible with shareLambdaForm

[cjjdespres]

If -Xshareclasses:shareLambdaForm is specified when running an acmeair client with JITServer, the client will segfault (if -Xrs is enabled) or start registering java null pointer exceptions in its logs (otherwise). I was able to limit the client to compiling a single method java/util/concurrent/ConcurrentLinkedQueue.updateHead(Ljava/util/concurrent/ConcurrentLinkedQueue$Node;Ljava/util/concurrent/ConcurrentLinkedQueue$Node;)V and still get a crash. Comparing a local compilation to a remote one, the first difference is here:

Local:
Object table:
  obj7     0000000000B5B5E0   000000060E3CD600 82d76a38   java/lang/invoke/DirectMethodHandle
  obj10    0000000000B5B5F8   000000060E3CD700 9ffd99dd   java/lang/invoke/DirectMethodHandle

Symbols:
#487:  java/lang/invoke/LambdaForm$DMH/0x0000000000000000._D_1 Ljava/lang/invoke/MethodHandle;[ final Static] (obj7) [flags 0x20307 0x0 ] [0x7fc30a2230f0] (Address)
#569:  java/lang/invoke/LambdaForm$DMH/0x0000000000000000._D_1 Ljava/lang/invoke/MethodHandle;[ final Static] (obj10) [flags 0x20307 0x0 ] [0x7fc30a2833f0] (Address)

----------------------------------------------------------------------------------------

Remote:
Object table:
  obj7     0000000000B5B5E0   000000060E3CD600 82d76a38   java/lang/invoke/DirectMethodHandle
  obj10    0000000000B5B5F8   000000060E3CD700 9ffd99dd   java/lang/invoke/DirectMethodHandle

Symbols:
#487:  java/lang/invoke/LambdaForm$DMH/0x0000000000000000._D_1 Ljava/lang/invoke/MethodHandle;[ final Static] (obj7) [flags 0x20307 0x0 ] [0x7ff7c30df0d0] (Address)
<no second symbol>

In the JITServer compilation, there is a single symref #487 used twice in the IL for different aload nodes, whereas in the local compilation there are two symrefs, each used once. These are obtained by J9::SymbolReferenceTable::findOrCreateStaticSymbol(), and the methods:

openj9/runtime/compiler/env/j9method.cpp

Line 7281 in f6e6334

TR_ResolvedJ9Method::staticsAreSame(I_32 cpIndex1, TR_ResolvedMethod * m2, I_32 cpIndex2, bool &sigSame)

and

openj9/runtime/compiler/env/j9methodServer.cpp

Line 478 in f6e6334

TR_ResolvedJ9JITServerMethod::staticsAreSame(int32_t cpIndex1, TR_ResolvedMethod *m2, int32_t cpIndex2, bool &sigSame)

will be involved in determining if the statics are the same or not. The JITServer version skips checking if the field addresses of the references are the same, and relies solely on name comparisons. This used to be okay, but with shareLambdaForm there will be methods (lambda forms) that have identical names and yet constitute distinct static references.

If I delete the address comparison portion of the local version, I do start getting crashes without using JITServer, so this does seem to be what's causing the problem. (A problem, at least). The straightforward way of solving this issue would be to ask the client if the statics are the same or not in this situation. It would be good to avoid a new message, of course.

[cjjdespres]

Attn @mpirvu.

[mpirvu]

We can always send a message from the server to client to find the right answer. However, this may mean too many messages.
We can restrict the number of message by sending a message only when

• The complete names match (class/methodname/signature) AND
• When the class name indicates we are dealing with a lambda form (i.e. starts with java/lang/invoke/LambdaForm)

We should measure how many more messages we send with this approach in AcmeAir with Java21

[mpirvu]

I see that the frontend query that ilgen uses resolves to TR_J9VMBase::jitStaticsAreSame(TR_ResolvedMethod *method1, I_32 cpIndex1, TR_ResolvedMethod *method2, I_32 cpIndex2) which is a bit more complicated and may involve calling jitFieldsAreIdentical which for JITServer may send JITServer::MessageType::VM_jitFieldsOrStaticsAreSame.
Currently VM_jitFieldsOrStaticsAreSame represents 0.9% of all messages sent.

[cjjdespres]

It is a bit more complicated. I think having staticsAreSame return false for fields involving lambda forms would work, since we'd fall through to the jitFieldsOrStaticsAreIdentical case. In TR_J9ByteCodeIlGenerator::walker() it looks like we preemptively use cacheFields so jitFieldsOrStaticsAreIdentical can use the cached data, so it might not end up increasing the number of messages very much.

[cjjdespres]

Having staticsAreSame return false for lambda forms does seem to work - the client does not crash, and in the one method I've been examining there are now two symrefs for the two different lambda forms, matching the local compilation.

Just to get a sense of the difference in messages, I ran two acmeair clients until remote method compilations had stopped, more or less. One was run without lambda form sharing, and one was run with sharing and this fix:

Message statistics without sharing:
#0000    4899		compilationCode
#0001      44		compilationFailure
#0074   49646		VM_jitFieldsOrStaticsAreSame
- average 10 VM_jitFieldsOrStaticsAreSame messages per compilation attempt

Messages statistics with sharing+fix:
#0000    4879		compilationCode
#0001      45		compilationFailure
#0074   48795		VM_jitFieldsOrStaticsAreSame
- average 9.9 VM_jitFieldsOrStaticsAreSame messages per compilation attempt

So the number of those messages was more or less unchanged, at least in the two runs I did.

[mpirvu]

Nice, let's open a PR with this solution.

[cjjdespres]

This should be resolved by #20845.