Skip to content

Commit 3bc18fb

Browse files
authored
Improve performance of the parser's failure path (#374)
* Previously, an exception was instantiated on each ParseError and the parsing failure was completely dominated by the 'fillInStacktrace' method * Now all exception messages are aggregated into a single message and only one instance of ParseException is thrown * An improvement on the basic benchmark is ~300%
1 parent 4470516 commit 3bc18fb

File tree

1 file changed

+25
-9
lines changed
  • core/common/src/internal/format/parser

1 file changed

+25
-9
lines changed

core/common/src/internal/format/parser/Parser.kt

+25-9
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ internal fun <T> List<ParserStructure<T>>.concat(): ParserStructure<T> {
4848
} else {
4949
ParserStructure(operations, followedBy.map { it.append(other) })
5050
}
51+
5152
fun <T> ParserStructure<T>.simplify(): ParserStructure<T> {
5253
val newOperations = mutableListOf<ParserOperation<T>>()
5354
var currentNumberSpan: MutableList<NumberConsumer<T>>? = null
@@ -122,7 +123,7 @@ internal interface Copyable<Self> {
122123
}
123124

124125
@JvmInline
125-
internal value class Parser<Output: Copyable<Output>>(
126+
internal value class Parser<Output : Copyable<Output>>(
126127
private val commands: ParserStructure<Output>
127128
) {
128129
/**
@@ -145,7 +146,7 @@ internal value class Parser<Output: Copyable<Output>>(
145146
onSuccess: (Int, Output) -> Unit
146147
) {
147148
val parseOptions = mutableListOf(ParserState(initialContainer, commands, startIndex))
148-
iterate_over_alternatives@while (true) {
149+
iterate_over_alternatives@ while (true) {
149150
val state = parseOptions.removeLastOrNull() ?: break
150151
val output = state.output.copy()
151152
var inputPosition = state.inputPosition
@@ -178,14 +179,15 @@ internal value class Parser<Output: Copyable<Output>>(
178179
fun match(input: CharSequence, initialContainer: Output, startIndex: Int = 0): Output {
179180
val errors = mutableListOf<ParseError>()
180181
parse(input, startIndex, initialContainer, allowDanglingInput = false, { errors.add(it) }, { _, out -> return@match out })
182+
/*
183+
* We do care about **all** parser errors and provide diagnostic information to make the error message approacheable
184+
* for authors of non-trivial formatters with a multitude of potential parsing paths.
185+
* For that, we sort errors so that the most successful parsing paths are at the top, and
186+
* add them all to the parse exception message.
187+
*/
181188
errors.sortByDescending { it.position }
182189
// `errors` can not be empty because each parser will have (successes + failures) >= 1, and here, successes == 0
183-
ParseException(errors.first()).let {
184-
for (error in errors.drop(1)) {
185-
it.addSuppressed(ParseException(error))
186-
}
187-
throw it
188-
}
190+
throw ParseException(errors)
189191
}
190192

191193
fun matchOrNull(input: CharSequence, initialContainer: Output, startIndex: Int = 0): Output? {
@@ -200,4 +202,18 @@ internal value class Parser<Output: Copyable<Output>>(
200202
)
201203
}
202204

203-
internal class ParseException(error: ParseError) : Exception("Position ${error.position}: ${error.message()}")
205+
internal class ParseException(errors: List<ParseError>) : Exception(formatError(errors))
206+
207+
private fun formatError(errors: List<ParseError>): String {
208+
if (errors.size == 1) {
209+
return "Position ${errors[0].position}: ${errors[0].message()}"
210+
}
211+
// 20 For average error string: "Expected X but got Y"
212+
// 13 for static part "Position :,"
213+
val averageMessageLength = 20 + 13
214+
return errors.joinTo(
215+
StringBuilder(averageMessageLength * errors.size),
216+
prefix = "Errors: ",
217+
separator = ", "
218+
) { "position ${it.position}: '${it.message()}'" }.toString()
219+
}

0 commit comments

Comments
 (0)