Skip to content

Commit 2520d34

Browse files
qwwdfsadmeta-codesync[bot]
authored andcommitted
Avoid redundant allocations (Kotlin#620)
Summary: * Optimizes work with `PsiElement`: no direct `text` access where appropriate as it's relatively expensive * Some early returns to avoid String copying when it is not necessary * Around 6-7% performance improvements on my machine on sequential `kotlinx.coroutines` runs (5 -> 4.65 sec in sequantial mode) Partially addresses Kotlin#619 Pull Request resolved: Kotlin#620 Reviewed By: cortinico Differential Revision: D107013190 Pulled By: hick209 fbshipit-source-id: 8a420697a5d5bbe708e46d016c0042b9d2fd3a71
1 parent e913a0b commit 2520d34

8 files changed

Lines changed: 35 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
1414

1515
### Changed
1616

17+
* Reduced overall number of allocations to improve formatting performance (~6-7%) (https://github.com/facebook/ktfmt/pull/620)
1718

1819
## [0.63]
1920

core/src/main/java/com/facebook/ktfmt/format/KotlinInput.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,11 @@ class KotlinInput(private val text: String, file: KtFile) : Input() {
130130
)
131131
}
132132

133-
private fun makePositionToColumnMap(toks: List<KotlinTok>) =
134-
ImmutableMap.copyOf(toks.map { it.position to it.column }.toMap())
133+
private fun makePositionToColumnMap(toks: List<KotlinTok>): ImmutableMap<Int, Int> {
134+
val builder = ImmutableMap.builderWithExpectedSize<Int, Int>(toks.size)
135+
toks.forEach { builder.put(it.position, it.column) }
136+
return builder.build()
137+
}
135138

136139
private fun buildToks(file: KtFile, fileText: String): ImmutableList<KotlinTok> {
137140
val tokenizer = Tokenizer(fileText, file)

core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1619,7 +1619,7 @@ class KotlinInputAstVisitor(
16191619
private fun hasSourceNewlineInLambdaBody(lambdaExpression: KtLambdaExpression): Boolean {
16201620
val functionLiteral = lambdaExpression.functionLiteral
16211621
for (child in functionLiteral.node.children()) {
1622-
if (child.psi is PsiWhiteSpace && child.text.contains('\n')) return true
1622+
if (child.psi is PsiWhiteSpace && child.textContains('\n')) return true
16231623
}
16241624
return false
16251625
}

core/src/main/java/com/facebook/ktfmt/format/RedundantElementManager.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,12 @@ object RedundantElementManager {
7171
}
7272
)
7373

74-
val result = StringBuilder(code)
7574
val elementsToRemove =
7675
redundantSemicolonDetector.getRedundantSemicolonElements() +
7776
redundantImportDetector.getRedundantImportElements() +
7877
trailingCommaDetector.getTrailingCommaElements()
78+
if (elementsToRemove.isEmpty()) return code
79+
val result = StringBuilder(code)
7980

8081
for (element in elementsToRemove.sortedByDescending(PsiElement::endOffset)) {
8182
// Don't insert extra newlines when the semicolon is already a line terminator
@@ -108,8 +109,9 @@ object RedundantElementManager {
108109
}
109110
)
110111

111-
val result = StringBuilder(code)
112112
val suggestionElements = trailingCommaSuggestor.getTrailingCommaSuggestions()
113+
if (suggestionElements.isEmpty()) return code
114+
val result = StringBuilder(code)
113115

114116
for (element in suggestionElements.sortedByDescending(PsiElement::endOffset)) {
115117
result.insert(element.endOffset, ',')
@@ -120,6 +122,6 @@ object RedundantElementManager {
120122

121123
private fun PsiElement?.containsNewline(): Boolean {
122124
if (this !is PsiWhiteSpace) return false
123-
return this.text.contains('\n')
125+
return this.textContains('\n')
124126
}
125127
}

core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal class RedundantSemicolonDetector {
5252

5353
/** returns **true** if this element was an extra comma, **false** otherwise. */
5454
private fun isExtraSemicolon(element: PsiElement): Boolean {
55-
if (element.text != ";") {
55+
if (element !is LeafPsiElement || element.elementType != KtTokens.SEMICOLON) {
5656
return false
5757
}
5858

core/src/main/java/com/facebook/ktfmt/format/Tokenizer.kt

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,16 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
6161
}
6262

6363
override fun visitElement(element: PsiElement) {
64-
val startIndex = element.startOffset
65-
val endIndex = element.endOffset
66-
val elementText = element.text
67-
val originalText = fileText.substring(startIndex, endIndex)
64+
// Do not materialize text/text ranges when it's non needed -- e.g. for KtFile, composite
65+
// PsiElement(...) etc.
66+
val elementText by lazy(LazyThreadSafetyMode.NONE) { element.text }
67+
val originalText by
68+
lazy(LazyThreadSafetyMode.NONE) {
69+
fileText.substring(element.startOffset, element.endOffset)
70+
}
6871
when (element) {
6972
is PsiComment -> {
70-
if (element.text.startsWith("/*") && !element.text.endsWith("*/")) {
73+
if (elementText.startsWith("/*") && !elementText.endsWith("*/")) {
7174
throw ParseError(
7275
"Unclosed comment",
7376
StringUtil.offsetToLineColumn(fileText, element.startOffset),
@@ -87,7 +90,7 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
8790
index = index,
8891
originalText = originalText,
8992
text = elementText,
90-
position = startIndex,
93+
position = element.startOffset,
9194
column = 0,
9295
isToken = treatAsToken,
9396
kind = KtTokens.EOF,
@@ -105,7 +108,7 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
105108
originalText,
106109
),
107110
text = elementText,
108-
position = startIndex,
111+
position = element.startOffset,
109112
column = 0,
110113
isToken = true,
111114
kind = KtTokens.EOF,
@@ -124,11 +127,11 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
124127
index = -1,
125128
originalText =
126129
fileText.substring(
127-
startIndex + matcher.start(),
128-
startIndex + matcher.end(),
130+
element.startOffset + matcher.start(),
131+
element.startOffset + matcher.end(),
129132
),
130133
text = text,
131-
position = startIndex + matcher.start(),
134+
position = element.startOffset + matcher.start(),
132135
column = 0,
133136
isToken = false,
134137
kind = KtTokens.EOF,
@@ -141,7 +144,7 @@ class Tokenizer(private val fileText: String, val file: KtFile) : KtTreeVisitorV
141144
index = index,
142145
originalText = originalText,
143146
text = elementText,
144-
position = startIndex,
147+
position = element.startOffset,
145148
column = 0,
146149
isToken = true,
147150
kind = KtTokens.EOF,

core/src/main/java/com/facebook/ktfmt/format/TrailingCommas.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package com.facebook.ktfmt.format
1919
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
2020
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
2121
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
22+
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
23+
import org.jetbrains.kotlin.lexer.KtTokens
2224
import org.jetbrains.kotlin.psi.KtClassBody
2325
import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
2426
import org.jetbrains.kotlin.psi.KtElement
@@ -47,10 +49,9 @@ object TrailingCommas {
4749
}
4850

4951
private fun isTrailingComma(element: PsiElement): Boolean {
50-
if (element.text != ",") {
52+
if (element !is LeafPsiElement || element.elementType != KtTokens.COMMA) {
5153
return false
5254
}
53-
5455
return extractManagedList(element.parent)?.trailingComma == element
5556
}
5657
}
@@ -77,10 +78,6 @@ object TrailingCommas {
7778
* ```
7879
*/
7980
fun takeElement(element: KtElement) {
80-
if (!element.text.contains("\n")) {
81-
return // Only suggest trailing commas where there is already a line break
82-
}
83-
8481
when (element) {
8582
is KtEnumEntry, // Only suggest on the KtClassBody container
8683
is KtWhenEntry -> return
@@ -99,6 +96,9 @@ object TrailingCommas {
9996
}
10097

10198
val list = extractManagedList(element) ?: return
99+
if (!element.textContains('\n')) {
100+
return // Only suggest trailing commas where there is already a line break
101+
}
102102
if (list.items.size <= 1) {
103103
return // Never insert commas to single-element lists
104104
}

core/src/main/java/com/facebook/ktfmt/format/WhitespaceTombstones.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import java.util.regex.Pattern.MULTILINE
2222
object WhitespaceTombstones {
2323
/** See [replaceTrailingWhitespaceWithTombstone]. */
2424
const val SPACE_TOMBSTONE = '\u0003'
25+
private val TRAILING_WHITESPACE_PATTERN = Pattern.compile(" ($)", MULTILINE)
2526

2627
fun String.indexOfWhitespaceTombstone(): Int = this.indexOf(SPACE_TOMBSTONE)
2728

@@ -32,7 +33,7 @@ object WhitespaceTombstones {
3233
* replace it back to a space.
3334
*/
3435
fun replaceTrailingWhitespaceWithTombstone(s: String): String {
35-
return Pattern.compile(" ($)", MULTILINE).matcher(s).replaceAll("$SPACE_TOMBSTONE$1")
36+
return TRAILING_WHITESPACE_PATTERN.matcher(s).replaceAll("$SPACE_TOMBSTONE$1")
3637
}
3738

3839
/** See [replaceTrailingWhitespaceWithTombstone]. */

0 commit comments

Comments
 (0)