Skip to content

Commit

Permalink
Improve performance of the parser's failure path (#374)
Browse files Browse the repository at this point in the history
* 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%
  • Loading branch information
qwwdfsad committed Mar 26, 2024
1 parent 4470516 commit 3bc18fb
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions core/common/src/internal/format/parser/Parser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ internal fun <T> List<ParserStructure<T>>.concat(): ParserStructure<T> {
} else {
ParserStructure(operations, followedBy.map { it.append(other) })
}

fun <T> ParserStructure<T>.simplify(): ParserStructure<T> {
val newOperations = mutableListOf<ParserOperation<T>>()
var currentNumberSpan: MutableList<NumberConsumer<T>>? = null
Expand Down Expand Up @@ -122,7 +123,7 @@ internal interface Copyable<Self> {
}

@JvmInline
internal value class Parser<Output: Copyable<Output>>(
internal value class Parser<Output : Copyable<Output>>(
private val commands: ParserStructure<Output>
) {
/**
Expand All @@ -145,7 +146,7 @@ internal value class Parser<Output: Copyable<Output>>(
onSuccess: (Int, Output) -> Unit
) {
val parseOptions = mutableListOf(ParserState(initialContainer, commands, startIndex))
iterate_over_alternatives@while (true) {
iterate_over_alternatives@ while (true) {
val state = parseOptions.removeLastOrNull() ?: break
val output = state.output.copy()
var inputPosition = state.inputPosition
Expand Down Expand Up @@ -178,14 +179,15 @@ internal value class Parser<Output: Copyable<Output>>(
fun match(input: CharSequence, initialContainer: Output, startIndex: Int = 0): Output {
val errors = mutableListOf<ParseError>()
parse(input, startIndex, initialContainer, allowDanglingInput = false, { errors.add(it) }, { _, out -> return@match out })
/*
* We do care about **all** parser errors and provide diagnostic information to make the error message approacheable
* for authors of non-trivial formatters with a multitude of potential parsing paths.
* For that, we sort errors so that the most successful parsing paths are at the top, and
* add them all to the parse exception message.
*/
errors.sortByDescending { it.position }
// `errors` can not be empty because each parser will have (successes + failures) >= 1, and here, successes == 0
ParseException(errors.first()).let {
for (error in errors.drop(1)) {
it.addSuppressed(ParseException(error))
}
throw it
}
throw ParseException(errors)
}

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

internal class ParseException(error: ParseError) : Exception("Position ${error.position}: ${error.message()}")
internal class ParseException(errors: List<ParseError>) : Exception(formatError(errors))

private fun formatError(errors: List<ParseError>): String {
if (errors.size == 1) {
return "Position ${errors[0].position}: ${errors[0].message()}"
}
// 20 For average error string: "Expected X but got Y"
// 13 for static part "Position :,"
val averageMessageLength = 20 + 13
return errors.joinTo(
StringBuilder(averageMessageLength * errors.size),
prefix = "Errors: ",
separator = ", "
) { "position ${it.position}: '${it.message()}'" }.toString()
}

0 comments on commit 3bc18fb

Please sign in to comment.