Fixed
Status Update
No update yet.
Comments
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 20a59c33ba405e6749bd6fa80faa32e4c6fa370f
Author: Yigit Boyar <yboyar@google.com>
Date: Mon Nov 15 08:40:32 2021
Add XMethodElement#name to reflect source name
This CL adds back the name property to XMethodElement. Unlike jvmName,
this property refers to the name in source and might be different from
the JVM name.
Note that, for functions that reference a value class, kotlin generates
a name with - in it, which makes it invalid in java source. As such,
these methods don't show up in KAPT stubs but they do show up in
compiled code. We should probably address that inconsistency in
XProcessing eventually (
A followup CL will update Room to use name where it makes sense to fix
the internals bug.
Bug: 206517658
Bug: 205289020
Test: XExecutableElementTest
Change-Id: I79864ffdde24d7edd7f0c49f668efc920ace3857
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMethodElement.kt
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
https://android-review.googlesource.com/1892715
Branch: androidx-main
commit 20a59c33ba405e6749bd6fa80faa32e4c6fa370f
Author: Yigit Boyar <yboyar@google.com>
Date: Mon Nov 15 08:40:32 2021
Add XMethodElement#name to reflect source name
This CL adds back the name property to XMethodElement. Unlike jvmName,
this property refers to the name in source and might be different from
the JVM name.
Note that, for functions that reference a value class, kotlin generates
a name with - in it, which makes it invalid in java source. As such,
these methods don't show up in KAPT stubs but they do show up in
compiled code. We should probably address that inconsistency in
XProcessing eventually (
A followup CL will update Room to use name where it makes sense to fix
the internals bug.
Bug: 206517658
Bug: 205289020
Test: XExecutableElementTest
Change-Id: I79864ffdde24d7edd7f0c49f668efc920ace3857
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMethodElement.kt
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit cedb2ecd5806502ef5fff6f36907c06a138a68b1
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Nov 17 23:35:05 2021
Add configuration option to not filter out inlines
Original implementation of XProcessing always excluded methods that
receive value classes as parameters because they cannot be called from
java source. This is also what KAPT does when generating stubs.
This configuration allows room to not do the filtering, which increases
get declared methods performance ~100x.
Note that this does not help with collecting all methods as that still
requires type resolution. That being said, we can possibly slightly
improve it by avoiding some overrides checks (e.g. we can group by
parameter count or check for override modifier for kotlin sources).
Room still uses the slow configuration. We can followup with another
CL to see if we can make Room benefit from it.
stats:
GetMethodsScenarioTest > filterOutInline_declaredMethods STANDARD_OUT
Stats(min=92.546784ms, max=152.486889ms, avg=120.771651ms, mean=145.828504ms)
GetMethodsScenarioTest > keepInline_declaredMethods STANDARD_OUT
Stats(min=946.523us, max=1.731635ms, avg=1.301860ms, mean=1.441747ms)
GetMethodsScenarioTest > filterOutInline_enclosedElements STANDARD_OUT
Stats(min=81.061261ms, max=410.522507ms, avg=140.475732ms, mean=271.770458ms)
GetMethodsScenarioTest > keepInline_enclosedElements STANDARD_OUT
Stats(min=1.046700ms, max=1.678441ms, avg=1.308582ms, mean=1.192400ms)
Fixes: 204913321
Fixes: 206517658
Test: InternalModifierTest
Change-Id: I41c5d51db0981e1cf163e92a7e8405a83915a688
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XProcessingEnvConfig.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
A room/room-compiler/src/test/resources/META-INF/services/androidx.room.compiler.processing.XProcessingEnvironmentTestConfigProvider
M room/room-compiler/src/main/kotlin/androidx/room/DatabaseProcessingStep.kt
M room/scripts/profile.sh
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/profiling/ProfileRule.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt
A room/room-compiler/src/test/kotlin/androidx/room/RoomTestEnvConfigProvider.kt
M room/scripts/ksp-kapt-comparison.sh
M room/room-compiler/src/main/kotlin/androidx/room/RoomKspProcessor.kt
A room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/profiling/GetMethodsScenarioTest.kt
M room/room-compiler/src/main/kotlin/androidx/room/RoomProcessor.kt
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/InternalModifierTest.kt
https://android-review.googlesource.com/1896555
Branch: androidx-main
commit cedb2ecd5806502ef5fff6f36907c06a138a68b1
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Nov 17 23:35:05 2021
Add configuration option to not filter out inlines
Original implementation of XProcessing always excluded methods that
receive value classes as parameters because they cannot be called from
java source. This is also what KAPT does when generating stubs.
This configuration allows room to not do the filtering, which increases
get declared methods performance ~100x.
Note that this does not help with collecting all methods as that still
requires type resolution. That being said, we can possibly slightly
improve it by avoiding some overrides checks (e.g. we can group by
parameter count or check for override modifier for kotlin sources).
Room still uses the slow configuration. We can followup with another
CL to see if we can make Room benefit from it.
stats:
GetMethodsScenarioTest > filterOutInline_declaredMethods STANDARD_OUT
Stats(min=92.546784ms, max=152.486889ms, avg=120.771651ms, mean=145.828504ms)
GetMethodsScenarioTest > keepInline_declaredMethods STANDARD_OUT
Stats(min=946.523us, max=1.731635ms, avg=1.301860ms, mean=1.441747ms)
GetMethodsScenarioTest > filterOutInline_enclosedElements STANDARD_OUT
Stats(min=81.061261ms, max=410.522507ms, avg=140.475732ms, mean=271.770458ms)
GetMethodsScenarioTest > keepInline_enclosedElements STANDARD_OUT
Stats(min=1.046700ms, max=1.678441ms, avg=1.308582ms, mean=1.192400ms)
Fixes: 204913321
Fixes: 206517658
Test: InternalModifierTest
Change-Id: I41c5d51db0981e1cf163e92a7e8405a83915a688
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XProcessingEnvConfig.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
A room/room-compiler/src/test/resources/META-INF/services/androidx.room.compiler.processing.XProcessingEnvironmentTestConfigProvider
M room/room-compiler/src/main/kotlin/androidx/room/DatabaseProcessingStep.kt
M room/scripts/profile.sh
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/profiling/ProfileRule.kt
M room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt
A room/room-compiler/src/test/kotlin/androidx/room/RoomTestEnvConfigProvider.kt
M room/scripts/ksp-kapt-comparison.sh
M room/room-compiler/src/main/kotlin/androidx/room/RoomKspProcessor.kt
A room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/profiling/GetMethodsScenarioTest.kt
M room/room-compiler/src/main/kotlin/androidx/room/RoomProcessor.kt
M room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/InternalModifierTest.kt
Description
When a function/property references a value class, kotlin generates a jvm function that has a
-in it, which makes it intentionally invalid in java sources.As such, these methods don't show up in KAPT stubs but will show up if it is coming from a dependency (because it is valid in .class file).
We need to figure out what to do for them.
Similarly, in KSP, we do ignore properties w/ value class types (but appearantly we don't for methods). We need to figure out a consistent path for these.
Some options:
a) always show them. That is, if they are not in KAPT stubs, syhtesize from metadata (may not be possible)
b) always hide them in KAPT (even if it is coming from a dependency). This would make it hard to generate kotlin code in KAPT, but it is already hard and Room is unlikely to need it.
c) Keep them in KSP and keep KAPT as is. This might be the best option for code that is migrating to Xprocessing from KAPT but it is an inconsistent state.