necessary and sufficient conditions for inclusion in InnerClasses JVM attribute #24
Labels
Comments
|
I'm not going to work on this any time soon and this is low priority. |
|
I recently re-wrote how we compute the InnerClass table in scalac, this should just work for dotty, too. scala/scala#4807. |
|
See also the PRs linked in the original issue: scala/bug#6759 (comment), which include a thorough set of test cases. |
odersky
referenced
this issue
in dotty-staging/dotty
Jun 10, 2022
Some tests by Matt had cases where `.nn` figured prominently in the flamegraph.
I optimized it so that the fast path is streamlined and inlined.
In the following test code:
```
def x: String | Null = "abc"
val y = x.nn
```
the new implementation is
```
10: getstatic #20 // Field MODULE$:LTest$;
13: invokevirtual #24 // Method x:()Ljava/lang/String;
16: astore_0
17: getstatic #29 // Field scala/runtime/Scala3RunTime$.MODULE$:Lscala/runtime/Scala3RunTime$;
20: aload_0
21: invokevirtual #33 // Method scala/runtime/Scala3RunTime$.nn:(Ljava/lang/Object;)Ljava/lang/Object;
24: checkcast #35 // class java/lang/String
27: putstatic #37 // Field y:Ljava/lang/String;
```
The previous implementation was two bytes shorter, but contained in the critical path
- A call to a ScalaRuntime method, which was not inlineable by the JIT compiler due to its size,
- A conversion of a cmparison to a Boolean value (not sure this matters)
- A cast from the return type `Object` to the actual type.
bishabosha
referenced
this issue
in dotty-staging/dotty
Oct 18, 2022
Some tests by Matt had cases where `.nn` figured prominently in the flamegraph.
I optimized it so that the fast path is streamlined and inlined.
In the following test code:
```
def x: String | Null = "abc"
val y = x.nn
```
the new implementation is
```
10: getstatic #20 // Field MODULE$:LTest$;
13: invokevirtual #24 // Method x:()Ljava/lang/String;
16: astore_0
17: getstatic #29 // Field scala/runtime/Scala3RunTime$.MODULE$:Lscala/runtime/Scala3RunTime$;
20: aload_0
21: invokevirtual #33 // Method scala/runtime/Scala3RunTime$.nn:(Ljava/lang/Object;)Ljava/lang/Object;
24: checkcast #35 // class java/lang/String
27: putstatic #37 // Field y:Ljava/lang/String;
```
The previous implementation was two bytes shorter, but contained in the critical path
- A call to a ScalaRuntime method, which was not inlineable by the JIT compiler due to its size,
- A conversion of a cmparison to a Boolean value (not sure this matters)
- A cast from the return type `Object` to the actual type.
odersky
added a commit
that referenced
this issue
Jan 20, 2023
…dled-by-backend Add errorhandling stack handled by backend
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Emitting a class file involves populating its InnerClass table as per the JVM spec.
For that, GenBCode follows the same strategy GenASM does (which in turn follows what GenJVM did): intercepting via
asmClassType()each usage of a class-name in the class file being emitted.That gives the right answer about 99.9% of the time. However as https://issues.scala-lang.org/browse/SI-6759 explains, there are cases where that approach includes too much in the InnerClasses table.
GenBCode has a solution for that,
refreshInnerClasses(), which should be adopted as the primary mechanism (throwing away theinnerClassBufferASMthat's part of theBCInnerClassGentrait).The text was updated successfully, but these errors were encountered: