Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use StringBuilder in String.replace (2x faster) #3714

Closed
wants to merge 1 commit into from
Closed

Use StringBuilder in String.replace (2x faster) #3714

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 31 additions & 7 deletions libraries/stdlib/jvm/src/kotlin/text/StringsJVM.kt
View file
Open in desktop
Expand Up @@ -61,20 +61,44 @@ public actual fun String?.equals(other: String?, ignoreCase: Boolean = false): B
*/
@Suppress("ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS")
public actual fun String.replace(oldChar: Char, newChar: Char, ignoreCase: Boolean = false): String {
if (!ignoreCase)
return (this as java.lang.String).replace(oldChar, newChar)
else
return splitToSequence(oldChar, ignoreCase = ignoreCase).joinToString(separator = newChar.toString())
// prefer case-insensitive platform implementation
if (!ignoreCase) return (this as java.lang.String).replace(oldChar, newChar)

return buildString(length) {
this@replace.forEach { c ->
append(if (c.equals(oldChar, ignoreCase)) newChar else c)
}
}
}

/**
* Returns a new string obtained by replacing all occurrences of the [oldValue] substring in this string
* with the specified [newValue] string.
*/
@Suppress("ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS")
public actual fun String.replace(oldValue: String, newValue: String, ignoreCase: Boolean = false): String =
splitToSequence(oldValue, ignoreCase = ignoreCase).joinToString(separator = newValue)

public actual fun String.replace(oldValue: String, newValue: String, ignoreCase: Boolean = false): String {
// prefer case-insensitive platform implementation
if (!ignoreCase) return (this as java.lang.String).replace(oldValue, newValue)

var occurrenceIndex: Int = indexOf(oldValue, 0, ignoreCase)
// FAST PATH: no match
if (occurrenceIndex < 0) return this

val oldValueLength = oldValue.length
val searchStep = oldValueLength.coerceAtLeast(1)
val newLengthHint = length - oldValueLength + newValue.length
if (newLengthHint < 0) throw OutOfMemoryError()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a comment explaining how this overflow proves that it's impossible to build the desired string, and maybe even include some of the length information in a message in the OutOfMemoryError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a comment explaining how this overflow proves that it's impossible to build the desired string

Something like: "detect Int overflow"?

maybe even include some of the length information in a message in the OutOfMemoryError

Can you provide a suggestion?
Please consider that every append in this function can cause a similar error, this hint works wello only for a single replacement.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expand on it a bit more: "even a single replacement would create a String with more than Int.MAX_VALUE characters, not allowed"
With your point about subsequent replacements causing the same error, I suppose that the errors should appear uniform, so OutOfMemoryError() is fine.

val stringBuilder = StringBuilder(newLengthHint)

var i = 0
do {
stringBuilder.append(this, i, occurrenceIndex).append(newValue)
i = occurrenceIndex + oldValueLength
if (occurrenceIndex >= length) break
occurrenceIndex = indexOf(oldValue, occurrenceIndex + searchStep, ignoreCase)
} while (occurrenceIndex > 0)
return stringBuilder.append(this, i, length).toString()
}

/**
* Returns a new string with the first occurrence of [oldChar] replaced with [newChar].
Expand Down