jlengrand/kotlin
1
0
Fork 0
mirror of https://github.com/jlengrand/kotlin.git synced 2026-03-10 08:31:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

KT-14258 Optimize accesses to properties defined into companion

Browse Source 
- Use direct access to property defined into companion object when
it is possible rather than always use an accessor to access the
property.
- Use direct access will speedup runtime performance.
- Avoid to generate useless accessors for companion properties.

Fix of https://youtrack.jetbrains.com/issue/KT-14258
This commit is contained in:
Mikaël Peltier
2018-02-14 11:33:57 +01:00
committed by Alexander Udalov
parent fd244be9ca
commit d0ed0c4049
17 changed files with 220 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java
View File

@@ -1950,6 +1950,10 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
}
}
private static boolean isDefaultAccessor(@Nullable PropertyAccessorDescriptor accessor) {
return accessor == null || accessor.isDefault();
}
public StackValue.Property intermediateValueForProperty(
@NotNull PropertyDescriptor propertyDescriptor,
boolean forceField,
@@ -1976,7 +1980,9 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
fieldAccessorKind = FieldAccessorKind.LATEINIT_INTRINSIC;
}
else if (isBackingFieldInClassCompanion &&
(forceField || propertyDescriptor.isConst() && Visibilities.isPrivate(propertyDescriptor.getVisibility()))) {
(forceField ||
(Visibilities.isPrivate(propertyDescriptor.getVisibility()) &&
isDefaultAccessor(propertyDescriptor.getGetter()) && isDefaultAccessor(propertyDescriptor.getSetter())))) {
fieldAccessorKind = FieldAccessorKind.IN_CLASS_COMPANION;
}
else if ((syntheticBackingField &&
@@ -2005,6 +2011,10 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
boolean skipPropertyAccessors;
PropertyDescriptor originalPropertyDescriptor = DescriptorUtils.unwrapFakeOverride(propertyDescriptor);
boolean directAccessToGetter = couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context,
state.getShouldInlineConstVals());
boolean directAccessToSetter = couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context,
state.getShouldInlineConstVals());
if (fieldAccessorKind == FieldAccessorKind.LATEINIT_INTRINSIC) {
skipPropertyAccessors = !isPrivateProperty || context.getClassOrPackageParentContext() == backingFieldContext;
@@ -2016,8 +2026,9 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
ownerDescriptor = propertyDescriptor;
}
else if (fieldAccessorKind == FieldAccessorKind.IN_CLASS_COMPANION || fieldAccessorKind == FieldAccessorKind.FIELD_FROM_LOCAL) {
boolean isInlinedConst = propertyDescriptor.isConst() && state.getShouldInlineConstVals();
skipPropertyAccessors = isInlinedConst || !isPrivateProperty || skipAccessorsForPrivateFieldInOuterClass;
// Do not use accessor 'access<property name>$cp' when the property can be accessed directly by its backing field.
skipPropertyAccessors = skipAccessorsForPrivateFieldInOuterClass ||
(directAccessToGetter && (!propertyDescriptor.isVar() || directAccessToSetter));
if (!skipPropertyAccessors) {
//noinspection ConstantConditions
@@ -2038,23 +2049,22 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
}
if (!skipPropertyAccessors) {
if (!couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context, state.getShouldInlineConstVals())) {
if (!directAccessToGetter || !directAccessToSetter) {
propertyDescriptor = context.getAccessorForSuperCallIfNeeded(propertyDescriptor, superCallTarget, state);
propertyDescriptor = context.accessibleDescriptor(propertyDescriptor, superCallTarget);
PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
if (getter != null && !isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
}
}
if (propertyDescriptor.isVar()) {
PropertySetterDescriptor setter = propertyDescriptor.getSetter();
if (setter != null &&
!couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context, state.getShouldInlineConstVals()) &&
!isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
if (!isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
if (!directAccessToGetter) {
PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
if (getter != null) {
callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
}
}
if (propertyDescriptor.isVar() && !directAccessToSetter) {
PropertySetterDescriptor setter = propertyDescriptor.getSetter();
if (setter != null) {
callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
}
}
}
}
}

19
compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java
View File

@@ -118,11 +118,19 @@ public class JvmCodegenUtil {
!closure.isSuspend();
}
private static boolean isCallInsideSameClassAsDeclared(@NotNull CallableMemberDescriptor descriptor, @NotNull CodegenContext context) {
private static boolean isCallInsideSameClassAsFieldRepresentingProperty(
@NotNull PropertyDescriptor descriptor,
@NotNull CodegenContext context
) {
boolean isFakeOverride = descriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
boolean isDelegate = descriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;
DeclarationDescriptor containingDeclaration = descriptor.getContainingDeclaration().getOriginal();
if (JvmAbi.isPropertyWithBackingFieldInOuterClass(descriptor)) {
// For property with backed field, check if the access is done in the same class containing the backed field and
// not the class that declared the field.
containingDeclaration = containingDeclaration.getContainingDeclaration();
}
return !isFakeOverride && !isDelegate &&
(((context.hasThisDescriptor() && containingDeclaration == context.getThisDescriptor()) ||
@@ -188,12 +196,12 @@ public class JvmCodegenUtil {
if (KotlinTypeMapper.isAccessor(property)) return false;
CodegenContext context = contextBeforeInline.getFirstCrossInlineOrNonInlineContext();
// Inline functions can't use direct access because a field may not be visible at the call site
if (context.isInlineMethodContext()) {
// Inline functions or inline classes can't use direct access because a field may not be visible at the call site
if (context.isInlineMethodContext() || (context.getEnclosingClass() != null && context.getEnclosingClass().isInline())) {
return false;
}
if (!isCallInsideSameClassAsDeclared(property, context)) {
if (!isCallInsideSameClassAsFieldRepresentingProperty(property, context)) {
if (!isDebuggerContext(context)) {
// Unless we are evaluating expression in debugger context, only properties of the same class can be directly accessed
return false;
@@ -218,9 +226,6 @@ public class JvmCodegenUtil {
// Delegated and extension properties have no backing fields
if (isDelegated || property.getExtensionReceiverParameter() != null) return false;
// Companion object properties cannot be accessed directly because their backing fields are stored in the containing class
if (DescriptorUtils.isCompanionObject(property.getContainingDeclaration())) return false;
PropertyAccessorDescriptor accessor = forGetter ? property.getGetter() : property.getSetter();
// If there's no accessor declared we can use direct access

27
compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java
View File

@@ -130,14 +130,20 @@ public class PropertyCodegen {
genBackingFieldAndAnnotations(declaration, descriptor, false);
if (isAccessorNeeded(declaration, descriptor, getter)) {
boolean isDefaultGetterAndSetter = isDefaultAccessor(getter) && isDefaultAccessor(setter);
if (isAccessorNeeded(declaration, descriptor, getter, isDefaultGetterAndSetter)) {
generateGetter(declaration, descriptor, getter);
}
if (isAccessorNeeded(declaration, descriptor, setter)) {
if (isAccessorNeeded(declaration, descriptor, setter, isDefaultGetterAndSetter)) {
generateSetter(declaration, descriptor, setter);
}
}
private static boolean isDefaultAccessor(@Nullable KtPropertyAccessor accessor) {
return accessor == null || !accessor.hasBody();
}
private void genDestructuringDeclaration(
@NotNull KtDestructuringDeclarationEntry entry,
@NotNull PropertyDescriptor descriptor
@@ -194,11 +200,12 @@ public class PropertyCodegen {
private boolean isAccessorNeeded(
@Nullable KtProperty declaration,
@NotNull PropertyDescriptor descriptor,
@Nullable KtPropertyAccessor accessor
@Nullable KtPropertyAccessor accessor,
boolean isDefaultGetterAndSetter
) {
if (isConstOrHasJvmFieldAnnotation(descriptor)) return false;
boolean isDefaultAccessor = accessor == null || !accessor.hasBody();
boolean isDefaultAccessor = isDefaultAccessor(accessor);
// Don't generate accessors for interface properties with default accessors in DefaultImpls
if (kind == OwnerKind.DEFAULT_IMPLS && isDefaultAccessor) return false;
@@ -208,8 +215,16 @@ public class PropertyCodegen {
// Delegated or extension properties can only be referenced via accessors
if (declaration.hasDelegate() || declaration.getReceiverTypeReference() != null) return true;
// Companion object properties always should have accessors, because their backing fields are moved/copied to the outer class
if (isCompanionObject(descriptor.getContainingDeclaration())) return true;
// Companion object properties should have accessors for non-private properties because these properties can be referenced
// via getter/setter. But these accessors getter/setter are not required for private properties that have a default getter
// and setter, in this case, the property can always be accessed through the accessor 'access<property name>$cp' and avoid some
// useless indirection by using others accessors.
if (isCompanionObject(descriptor.getContainingDeclaration())) {
if (Visibilities.isPrivate(descriptor.getVisibility()) && isDefaultGetterAndSetter) {
return false;
}
return true;
}
// Non-const properties from multifile classes have accessors regardless of visibility
if (isNonConstTopLevelPropertyInMultifileClass(declaration, descriptor)) return true;

2
compiler/testData/asJava/lightClasses/compilationErrors/TraitClassObjectField.java vendored
View File

@@ -8,8 +8,6 @@ public interface TraitClassObjectField {
public static final java.lang.String x = "";
private static final java.lang.String y = "";
private final java.lang.String getY() { /* compiled code */ }
private Companion() { /* compiled code */ }
}
}

2
compiler/testData/asJava/lightClasses/nullabilityAnnotations/ClassObjectField.java vendored
View File

@@ -10,8 +10,6 @@ public final class ClassObjectField {
@org.jetbrains.annotations.Nullable
public final java.lang.String getX() { /* compiled code */ }
private final java.lang.String getY() { /* compiled code */ }
private Companion() { /* compiled code */ }
}
}

4
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt vendored
View File

@@ -16,7 +16,9 @@ class A {
fun `access$getBar$lp`(a: A): Int = 7
companion object {
private var foo = 1;
private var foo = 1
// Custom getter is needed, otherwise no need to generate getY and setY
get() = field
fun test() {
{

4
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.out vendored
View File

@@ -58,12 +58,12 @@ compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:18:15:
fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
companion object {
^
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:28:9: error: platform declaration clash: The following declarations have the same JVM signature (access$getFoo$p(LA$Companion;)I):
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:30:9: error: platform declaration clash: The following declarations have the same JVM signature (access$getFoo$p(LA$Companion;)I):
fun <get-foo>(): Int defined in A.Companion
fun `access$getFoo$p`(p: A.Companion): Int defined in A.Companion
fun `access$getFoo$p`(p: A.Companion): Int = 1
^
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:29:9: error: platform declaration clash: The following declarations have the same JVM signature (access$setFoo$p(LA$Companion;I)V):
compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:31:9: error: platform declaration clash: The following declarations have the same JVM signature (access$setFoo$p(LA$Companion;I)V):
fun <set-foo>(<set-?>: Int): Unit defined in A.Companion
fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
fun `access$setFoo$p`(p: A.Companion, d: Int) {}

16
compiler/testData/codegen/bytecodeText/companion/kt14258_1.kt vendored Normal file
View File

@@ -0,0 +1,16 @@
// Checks that methods 'access$getMy$p', 'access$getMy$cp' and 'getMy' are not generated and
// that backed field 'my' is directly used through a 'getstatic'
class My {
companion object {
private val my: String = "OK"
}
fun getMyValue() = my
}
// 1 GETSTATIC My.my
// 1 PUTSTATIC My.my
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
// 0 INVOKESTATIC My.access\$getMy\$cp
// 0 INVOKESPECIAL My\$Companion.getMy

20
compiler/testData/codegen/bytecodeText/companion/kt14258_2.kt vendored Normal file
View File

@@ -0,0 +1,20 @@
// Checks that methods 'access$getMy$p' and 'getMy' are not generated and
// that backed field 'my' is accessed through 'access$getMy$cp'
class My {
companion object {
private val my: String = "OK"
fun test(): String {
// accessor is required because field is move to Foo
return my
}
}
fun getMyValue() = test()
}
// 1 GETSTATIC My.my
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
// 1 INVOKESTATIC My.access\$getMy\$cp
// 0 INVOKESPECIAL My\$Companion.getMy

28
compiler/testData/codegen/bytecodeText/companion/kt14258_3.kt vendored Normal file
View File

@@ -0,0 +1,28 @@
// Checks that accessor methods are always used due to the overriding of the default setter of 'my' property.
class My {
companion object {
private var my: String = "OK"
set(value) { field = value }
}
fun getMyValue(): String {
// INVOKESTATIC My$Companion.access$setMy$p
my = "Overriden value"
// GETSTATIC My.my
return my
}
// PUTSTATIC My.my into clinit
// PUTSTATIC My.my into 'access$setMy$cp'
// GETSTATIC My.my into 'access$getMy$cp'
}
// 2 GETSTATIC My.my
// 2 PUTSTATIC My.my
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
// 1 INVOKESTATIC My\$Companion.access\$setMy\$p
// 1 INVOKESTATIC My.access\$setMy\$cp
// 1 INVOKESTATIC My.access\$getMy\$cp
// 1 INVOKESPECIAL My\$Companion.getMy
// 1 INVOKESPECIAL My\$Companion.setMy

23
compiler/testData/codegen/bytecodeText/companion/kt14258_4.kt vendored Normal file
View File

@@ -0,0 +1,23 @@
// Checks that accessor 'I$Companion.access$getBar\$p' is always used because the property is kept
// into the companion object.
interface I {
companion object {
private var bar = "Companion Field from I"
}
fun test(): String {
// INVOKESTATIC I$Companion.access$setBar$p
bar = "New value"
// INVOKESTATIC I$Companion.access$getBar$p
return bar
}
}
// 1 GETSTATIC I\$Companion.bar
// 2 PUTSTATIC I\$Companion.bar
// 1 INVOKESTATIC I\$Companion.access\$getBar\$p
// 1 INVOKESTATIC I\$Companion.access\$setBar\$p
// 0 INVOKESTATIC I\$Companion.access\$setBar\$cp
// 0 INVOKESPECIAL I\$Companion.getBar
// 0 INVOKESPECIAL I\$Companion.setBar

19
compiler/testData/codegen/bytecodeText/companion/kt14258_5.kt vendored Normal file
View File

@@ -0,0 +1,19 @@
// Checks that accessor are not used because property can be accessed directly.
interface I {
companion object {
private var bar = "Companion Field from I"
fun test(): String {
bar = "New value"
return bar
}
}
}
// 1 GETSTATIC I\$Companion.bar
// 2 PUTSTATIC I\$Companion.bar
// 0 INVOKESTATIC I\$Companion.access\$getBar\$p
// 0 INVOKESTATIC I\$Companion.access\$setBar\$cp
// 0 INVOKESPECIAL I\$Companion.getBar
// 0 INVOKESPECIAL I\$Companion.setBar

6
compiler/testData/codegen/bytecodeText/constProperty/accessorsForPrivateConstants.kt vendored
View File

@@ -9,9 +9,13 @@ class Foo {
}
fun foo() {
// Access to the property use getstatic on the backed field
LOCAL_PRIVATE
// Access to the property requires an invokestatic
OUTER_PRIVATE
}
}
// 2 INVOKESTATIC
// 1 INVOKESTATIC
// 1 PUTSTATIC
// 2 GETSTATIC

8
compiler/testData/codegen/bytecodeText/staticFields/classObjectSyntheticAccessor.kt vendored
View File

@@ -1,8 +0,0 @@
class A {
companion object {
private var r: Int = 1;
}
}
// A and companion object constructor call
// 3 ALOAD 0
// 1 synthetic access\$getR

2
compiler/testData/writeFlags/function/classObjectPrivate/privateVal.kt vendored
View File

@@ -4,6 +4,8 @@ class Foo {
companion object {
private val test = "String"
// Custom getter is needed, otherwise no need to generate getTest
get() = field
}
}

2
compiler/testData/writeFlags/function/classObjectPrivate/privateVar.kt vendored
View File

@@ -4,6 +4,8 @@ class Foo {
companion object {
private var test = "String"
// Custom setter is needed, otherwise no need to generate setTest
set(v) { field = v }
}
}

45
compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java generated
View File

@@ -853,6 +853,45 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/companion")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class Companion extends AbstractBytecodeTextTest {
public void testAllFilesPresentInCompanion() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/companion"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
}
@TestMetadata("kt14258_1.kt")
public void testKt14258_1() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_1.kt");
doTest(fileName);
}
@TestMetadata("kt14258_2.kt")
public void testKt14258_2() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_2.kt");
doTest(fileName);
}
@TestMetadata("kt14258_3.kt")
public void testKt14258_3() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_3.kt");
doTest(fileName);
}
@TestMetadata("kt14258_4.kt")
public void testKt14258_4() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_4.kt");
doTest(fileName);
}
@TestMetadata("kt14258_5.kt")
public void testKt14258_5() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_5.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/conditions")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -2601,12 +2640,6 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
doTest(fileName);
}
@TestMetadata("classObjectSyntheticAccessor.kt")
public void testClassObjectSyntheticAccessor() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/staticFields/classObjectSyntheticAccessor.kt");
doTest(fileName);
}
@TestMetadata("object.kt")
public void testObject() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/staticFields/object.kt");