Skip to content

Commit d0ed0c4

Browse files
Mikaël Peltierudalov
Mikaël Peltier
authored and
udalov
committed
KT-14258 Optimize accesses to properties defined into companion
- 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
1 parent fd244be commit d0ed0c4

File tree

17 files changed

+220
-53
lines changed

17 files changed

+220
-53
lines changed

‎compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,6 +1950,10 @@ private CodegenContext getBackingFieldContext(
19501950
}
19511951
}
19521952

1953+
private static boolean isDefaultAccessor(@Nullable PropertyAccessorDescriptor accessor) {
1954+
return accessor == null || accessor.isDefault();
1955+
}
1956+
19531957
public StackValue.Property intermediateValueForProperty(
19541958
@NotNull PropertyDescriptor propertyDescriptor,
19551959
boolean forceField,
@@ -1976,7 +1980,9 @@ public StackValue.Property intermediateValueForProperty(
19761980
fieldAccessorKind = FieldAccessorKind.LATEINIT_INTRINSIC;
19771981
}
19781982
else if (isBackingFieldInClassCompanion &&
1979-
(forceField || propertyDescriptor.isConst() && Visibilities.isPrivate(propertyDescriptor.getVisibility()))) {
1983+
(forceField ||
1984+
(Visibilities.isPrivate(propertyDescriptor.getVisibility()) &&
1985+
isDefaultAccessor(propertyDescriptor.getGetter()) && isDefaultAccessor(propertyDescriptor.getSetter())))) {
19801986
fieldAccessorKind = FieldAccessorKind.IN_CLASS_COMPANION;
19811987
}
19821988
else if ((syntheticBackingField &&
@@ -2005,6 +2011,10 @@ else if ((syntheticBackingField &&
20052011
boolean skipPropertyAccessors;
20062012

20072013
PropertyDescriptor originalPropertyDescriptor = DescriptorUtils.unwrapFakeOverride(propertyDescriptor);
2014+
boolean directAccessToGetter = couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context,
2015+
state.getShouldInlineConstVals());
2016+
boolean directAccessToSetter = couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context,
2017+
state.getShouldInlineConstVals());
20082018

20092019
if (fieldAccessorKind == FieldAccessorKind.LATEINIT_INTRINSIC) {
20102020
skipPropertyAccessors = !isPrivateProperty || context.getClassOrPackageParentContext() == backingFieldContext;
@@ -2016,8 +2026,9 @@ else if ((syntheticBackingField &&
20162026
ownerDescriptor = propertyDescriptor;
20172027
}
20182028
else if (fieldAccessorKind == FieldAccessorKind.IN_CLASS_COMPANION || fieldAccessorKind == FieldAccessorKind.FIELD_FROM_LOCAL) {
2019-
boolean isInlinedConst = propertyDescriptor.isConst() && state.getShouldInlineConstVals();
2020-
skipPropertyAccessors = isInlinedConst || !isPrivateProperty || skipAccessorsForPrivateFieldInOuterClass;
2029+
// Do not use accessor 'access<property name>$cp' when the property can be accessed directly by its backing field.
2030+
skipPropertyAccessors = skipAccessorsForPrivateFieldInOuterClass ||
2031+
(directAccessToGetter && (!propertyDescriptor.isVar() || directAccessToSetter));
20212032

20222033
if (!skipPropertyAccessors) {
20232034
//noinspection ConstantConditions
@@ -2038,23 +2049,22 @@ else if (fieldAccessorKind == FieldAccessorKind.IN_CLASS_COMPANION || fieldAcces
20382049
}
20392050

20402051
if (!skipPropertyAccessors) {
2041-
if (!couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context, state.getShouldInlineConstVals())) {
2052+
if (!directAccessToGetter || !directAccessToSetter) {
20422053
propertyDescriptor = context.getAccessorForSuperCallIfNeeded(propertyDescriptor, superCallTarget, state);
2043-
20442054
propertyDescriptor = context.accessibleDescriptor(propertyDescriptor, superCallTarget);
2045-
2046-
PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
2047-
if (getter != null && !isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
2048-
callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
2049-
}
2050-
}
2051-
2052-
if (propertyDescriptor.isVar()) {
2053-
PropertySetterDescriptor setter = propertyDescriptor.getSetter();
2054-
if (setter != null &&
2055-
!couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context, state.getShouldInlineConstVals()) &&
2056-
!isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
2057-
callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
2055+
if (!isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
2056+
if (!directAccessToGetter) {
2057+
PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
2058+
if (getter != null) {
2059+
callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
2060+
}
2061+
}
2062+
if (propertyDescriptor.isVar() && !directAccessToSetter) {
2063+
PropertySetterDescriptor setter = propertyDescriptor.getSetter();
2064+
if (setter != null) {
2065+
callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
2066+
}
2067+
}
20582068
}
20592069
}
20602070
}

‎compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,19 @@ public static boolean isConst(@NotNull CalculatedClosure closure) {
118118
!closure.isSuspend();
119119
}
120120

121-
private static boolean isCallInsideSameClassAsDeclared(@NotNull CallableMemberDescriptor descriptor, @NotNull CodegenContext context) {
121+
private static boolean isCallInsideSameClassAsFieldRepresentingProperty(
122+
@NotNull PropertyDescriptor descriptor,
123+
@NotNull CodegenContext context
124+
) {
122125
boolean isFakeOverride = descriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
123126
boolean isDelegate = descriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;
124127

125128
DeclarationDescriptor containingDeclaration = descriptor.getContainingDeclaration().getOriginal();
129+
if (JvmAbi.isPropertyWithBackingFieldInOuterClass(descriptor)) {
130+
// For property with backed field, check if the access is done in the same class containing the backed field and
131+
// not the class that declared the field.
132+
containingDeclaration = containingDeclaration.getContainingDeclaration();
133+
}
126134

127135
return !isFakeOverride && !isDelegate &&
128136
(((context.hasThisDescriptor() && containingDeclaration == context.getThisDescriptor()) ||
@@ -188,12 +196,12 @@ public static boolean couldUseDirectAccessToProperty(
188196
if (KotlinTypeMapper.isAccessor(property)) return false;
189197

190198
CodegenContext context = contextBeforeInline.getFirstCrossInlineOrNonInlineContext();
191-
// Inline functions can't use direct access because a field may not be visible at the call site
192-
if (context.isInlineMethodContext()) {
199+
// Inline functions or inline classes can't use direct access because a field may not be visible at the call site
200+
if (context.isInlineMethodContext() || (context.getEnclosingClass() != null && context.getEnclosingClass().isInline())) {
193201
return false;
194202
}
195203

196-
if (!isCallInsideSameClassAsDeclared(property, context)) {
204+
if (!isCallInsideSameClassAsFieldRepresentingProperty(property, context)) {
197205
if (!isDebuggerContext(context)) {
198206
// Unless we are evaluating expression in debugger context, only properties of the same class can be directly accessed
199207
return false;
@@ -218,9 +226,6 @@ public static boolean couldUseDirectAccessToProperty(
218226
// Delegated and extension properties have no backing fields
219227
if (isDelegated || property.getExtensionReceiverParameter() != null) return false;
220228

221-
// Companion object properties cannot be accessed directly because their backing fields are stored in the containing class
222-
if (DescriptorUtils.isCompanionObject(property.getContainingDeclaration())) return false;
223-
224229
PropertyAccessorDescriptor accessor = forGetter ? property.getGetter() : property.getSetter();
225230

226231
// If there's no accessor declared we can use direct access

‎compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,20 @@ private void gen(
130130

131131
genBackingFieldAndAnnotations(declaration, descriptor, false);
132132

133-
if (isAccessorNeeded(declaration, descriptor, getter)) {
133+
boolean isDefaultGetterAndSetter = isDefaultAccessor(getter) && isDefaultAccessor(setter);
134+
135+
if (isAccessorNeeded(declaration, descriptor, getter, isDefaultGetterAndSetter)) {
134136
generateGetter(declaration, descriptor, getter);
135137
}
136-
if (isAccessorNeeded(declaration, descriptor, setter)) {
138+
if (isAccessorNeeded(declaration, descriptor, setter, isDefaultGetterAndSetter)) {
137139
generateSetter(declaration, descriptor, setter);
138140
}
139141
}
140142

143+
private static boolean isDefaultAccessor(@Nullable KtPropertyAccessor accessor) {
144+
return accessor == null || !accessor.hasBody();
145+
}
146+
141147
private void genDestructuringDeclaration(
142148
@NotNull KtDestructuringDeclarationEntry entry,
143149
@NotNull PropertyDescriptor descriptor
@@ -194,11 +200,12 @@ private void genBackingFieldAndAnnotations(
194200
private boolean isAccessorNeeded(
195201
@Nullable KtProperty declaration,
196202
@NotNull PropertyDescriptor descriptor,
197-
@Nullable KtPropertyAccessor accessor
203+
@Nullable KtPropertyAccessor accessor,
204+
boolean isDefaultGetterAndSetter
198205
) {
199206
if (isConstOrHasJvmFieldAnnotation(descriptor)) return false;
200207

201-
boolean isDefaultAccessor = accessor == null || !accessor.hasBody();
208+
boolean isDefaultAccessor = isDefaultAccessor(accessor);
202209

203210
// Don't generate accessors for interface properties with default accessors in DefaultImpls
204211
if (kind == OwnerKind.DEFAULT_IMPLS && isDefaultAccessor) return false;
@@ -208,8 +215,16 @@ private boolean isAccessorNeeded(
208215
// Delegated or extension properties can only be referenced via accessors
209216
if (declaration.hasDelegate() || declaration.getReceiverTypeReference() != null) return true;
210217

211-
// Companion object properties always should have accessors, because their backing fields are moved/copied to the outer class
212-
if (isCompanionObject(descriptor.getContainingDeclaration())) return true;
218+
// Companion object properties should have accessors for non-private properties because these properties can be referenced
219+
// via getter/setter. But these accessors getter/setter are not required for private properties that have a default getter
220+
// and setter, in this case, the property can always be accessed through the accessor 'access<property name>$cp' and avoid some
221+
// useless indirection by using others accessors.
222+
if (isCompanionObject(descriptor.getContainingDeclaration())) {
223+
if (Visibilities.isPrivate(descriptor.getVisibility()) && isDefaultGetterAndSetter) {
224+
return false;
225+
}
226+
return true;
227+
}
213228

214229
// Non-const properties from multifile classes have accessors regardless of visibility
215230
if (isNonConstTopLevelPropertyInMultifileClass(declaration, descriptor)) return true;

‎compiler/testData/asJava/lightClasses/compilationErrors/TraitClassObjectField.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ static final class Companion {
88
public static final java.lang.String x = "";
99
private static final java.lang.String y = "";
1010

11-
private final java.lang.String getY() { /* compiled code */ }
12-
1311
private Companion() { /* compiled code */ }
1412
}
1513
}

‎compiler/testData/asJava/lightClasses/nullabilityAnnotations/ClassObjectField.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ public static final class Companion {
1010
@org.jetbrains.annotations.Nullable
1111
public final java.lang.String getX() { /* compiled code */ }
1212

13-
private final java.lang.String getY() { /* compiled code */ }
14-
1513
private Companion() { /* compiled code */ }
1614
}
1715
}

‎compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ class A {
1616
fun `access$getBar$lp`(a: A): Int = 7
1717

1818
companion object {
19-
private var foo = 1;
19+
private var foo = 1
20+
// Custom getter is needed, otherwise no need to generate getY and setY
21+
get() = field
2022

2123
fun test() {
2224
{

‎compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:18:15:
5858
fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
5959
companion object {
6060
^
61-
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):
61+
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):
6262
fun <get-foo>(): Int defined in A.Companion
6363
fun `access$getFoo$p`(p: A.Companion): Int defined in A.Companion
6464
fun `access$getFoo$p`(p: A.Companion): Int = 1
6565
^
66-
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):
66+
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):
6767
fun <set-foo>(<set-?>: Int): Unit defined in A.Companion
6868
fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
6969
fun `access$setFoo$p`(p: A.Companion, d: Int) {}

‎compiler/testData/codegen/bytecodeText/companion/kt14258_1.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Checks that methods 'access$getMy$p', 'access$getMy$cp' and 'getMy' are not generated and
2+
// that backed field 'my' is directly used through a 'getstatic'
3+
4+
class My {
5+
companion object {
6+
private val my: String = "OK"
7+
}
8+
9+
fun getMyValue() = my
10+
}
11+
12+
// 1 GETSTATIC My.my
13+
// 1 PUTSTATIC My.my
14+
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
15+
// 0 INVOKESTATIC My.access\$getMy\$cp
16+
// 0 INVOKESPECIAL My\$Companion.getMy

‎compiler/testData/codegen/bytecodeText/companion/kt14258_2.kt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Checks that methods 'access$getMy$p' and 'getMy' are not generated and
2+
// that backed field 'my' is accessed through 'access$getMy$cp'
3+
4+
class My {
5+
companion object {
6+
private val my: String = "OK"
7+
8+
fun test(): String {
9+
// accessor is required because field is move to Foo
10+
return my
11+
}
12+
}
13+
14+
fun getMyValue() = test()
15+
}
16+
17+
// 1 GETSTATIC My.my
18+
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
19+
// 1 INVOKESTATIC My.access\$getMy\$cp
20+
// 0 INVOKESPECIAL My\$Companion.getMy

‎compiler/testData/codegen/bytecodeText/companion/kt14258_3.kt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Checks that accessor methods are always used due to the overriding of the default setter of 'my' property.
2+
3+
class My {
4+
companion object {
5+
private var my: String = "OK"
6+
set(value) { field = value }
7+
}
8+
9+
fun getMyValue(): String {
10+
// INVOKESTATIC My$Companion.access$setMy$p
11+
my = "Overriden value"
12+
// GETSTATIC My.my
13+
return my
14+
}
15+
16+
// PUTSTATIC My.my into clinit
17+
// PUTSTATIC My.my into 'access$setMy$cp'
18+
// GETSTATIC My.my into 'access$getMy$cp'
19+
}
20+
21+
// 2 GETSTATIC My.my
22+
// 2 PUTSTATIC My.my
23+
// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
24+
// 1 INVOKESTATIC My\$Companion.access\$setMy\$p
25+
// 1 INVOKESTATIC My.access\$setMy\$cp
26+
// 1 INVOKESTATIC My.access\$getMy\$cp
27+
// 1 INVOKESPECIAL My\$Companion.getMy
28+
// 1 INVOKESPECIAL My\$Companion.setMy

0 commit comments

Comments
 (0)