Move static code used at runtime by aot compiled modules to a separate package #552
Conversation
| @@ -0,0 +1,6 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Sneaked this in, can remove if requested.
There was a problem hiding this comment.
I like this. Is there a way to run the tests from Maven without the diffs popping up individually in IntelliJ?
There was a problem hiding this comment.
I dug through the docs and found you can set APPROVAL_TESTS_USE_REPORTER=AutoApproveReporter which seems to work when I tried from IntelliJ. So maybe we don't need this script?
There was a problem hiding this comment.
Thanks for looking into it, I remove the script in this case.
But I need to add this note somewhere in the docs, otherwise I'm going to forget pretty quickly 😅
There was a problem hiding this comment.
Is there a way to run the tests from Maven without the diffs popping up individually in IntelliJ?
It was happening to me on a Mac, now on Linux is I don't have this horrific behavior anymore, but I can't recall if I did something about it ...
andreaTP
force-pushed
the
split-aot-runtime
branch
from
ffdf8a8
to
b2ae8ba
Compare
There was a problem hiding this comment.
We could drop this annotation now. Previously, we had referenced methods in multiple places, and referenced them directly in the generated bytecode. Now that we're using Java reflection getMethod() to reference them, IntelliJ understands that the methods are used.
It was useful when I first added it, but isn't needed now.
| @@ -104,7 +73,7 @@ public static void ELEM_DROP(AotContext ctx, AnnotatedInstruction ins, MethodVis | |||
| asm.visitVarInsn(Opcodes.ALOAD, ctx.instanceSlot()); | |||
| asm.visitLdcInsn(index); | |||
| asm.visitInsn(Opcodes.ACONST_NULL); | |||
| emitInvokeVirtual(asm, INSTANCE_SET_ELEMENT); | |||
| emitInvokeVirtual(asm, AotMethodsRefs.INSTANCE_SET_ELEMENT); | |||
There was a problem hiding this comment.
Let's keep these static imported. It looks cleaner and makes the diff smaller.
There was a problem hiding this comment.
In all honesty, I personally prefer this notation, but I'm not fussy on it.
|
|
||
| public final class AotMethods { | ||
| public final class AotMethodsRefs { |
There was a problem hiding this comment.
Let's name this AotMethodRefs to avoid the double plural. It also directly matches the English:
- AOT methods
- AOT method references
| @@ -0,0 +1,6 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
I like this. Is there a way to run the tests from Maven without the diffs popping up individually in IntelliJ?
andreaTP
force-pushed
the
split-aot-runtime
branch
from
b2ae8ba
to
154683d
Compare
Minor but "diff noisy" PR to get to #494