Fix multi parser performance regression

It was reported in https://github.com/sbt/sbt/issues/4890 that cosmetic
white space could cause problems for the paser. I tracked this down to
primarily being because of the
`val semi = token(OptSpace ~> ';' ~> OptSpace)` line. This would cause
excessive backtracking. I added a test for a multi line command with a
lot of cosmetic whitespace that was adapted from #4890 except that I
made it even more taxing by running adding 100 commands instead of the
roughly 10 in the report. Before the parser changes, the test would
more or less block indefinitely. I never saw it successfully complete.
After these changes, it completes in 30-50ms (which drops to about 2-3
ms if the number of commands is dropped from 100 to 3).

I verified manually in a different project that a number of different
multi command completions still worked. In particular, I tested that
`~foo/test; foo/tes` would expand to `~foo/test; foo/test` which is one
of the hardest cases to get right.

I also added a few extra test cases for the parser since I wasn't sure
what the impact of removing the OptSpace ~> from the semi parser would
be.
This commit is contained in:
Ethan Atkins 2019-07-27 21:40:48 -07:00
parent e3e2f29bdd
commit 196318e619
2 changed files with 23 additions and 4 deletions

View File

@ -155,15 +155,14 @@ object BasicCommands {
private[sbt] def multiParserImpl(state: Option[State]): Parser[List[String]] = {
val nonSemi = charClass(_ != ';', "not ';'")
val semi = token(OptSpace ~> ';' ~> OptSpace)
val semi = token(';')
val nonDelim = charClass(c => c != '"' && c != '{' && c != '}', label = "not '\"', '{', '}'")
val components = ((nonSemi & nonDelim) | StringEscapable | braces('{', '}')).+
val cmdPart = matched(components).examples()
val completionParser: Option[Parser[String]] =
state.map(s => OptSpace ~> matched(s.nonMultiParser) <~ OptSpace)
val cmdParser =
completionParser.map(sp => (sp & cmdPart) | cmdPart).getOrElse(cmdPart).map(_.trim)
state.map(s => OptSpace ~> matched((s.nonMultiParser & cmdPart) | cmdPart) <~ OptSpace)
val cmdParser = completionParser.getOrElse(cmdPart).map(_.trim)
val multiCmdParser: Parser[String] = semi ~> cmdParser
/*
* There are two cases that need to be handled separately:

View File

@ -7,6 +7,7 @@
package sbt
import scala.concurrent.duration._
import org.scalatest.FlatSpec
import sbt.internal.util.complete.Parser
@ -28,6 +29,15 @@ class MultiParserSpec extends FlatSpec {
}
it should "parse single command with leading spaces" in {
assert("; foo".parse == Seq("foo"))
assert(" ; foo".parse == Seq("foo"))
assert(" foo;".parse == Seq("foo"))
}
it should "parse single command with trailing spaces" in {
assert("; foo ".parse == Seq("foo"))
assert(";foo ".parse == Seq("foo"))
assert("foo; ".parse == Seq("foo"))
assert(" foo; ".parse == Seq("foo"))
assert(" foo ; ".parse == Seq("foo"))
}
it should "parse multiple commands with leading spaces" in {
assert("; foo;bar".parse == Seq("foo", "bar"))
@ -103,4 +113,14 @@ class MultiParserSpec extends FlatSpec {
val unmatchedEmptyBraces = "{{{{}}}"
assert(s"compile; $unmatchedEmptyBraces".parseEither.isLeft)
}
it should "handle cosmetic whitespace" in {
val commands = (1 to 100).map(_ => "compile")
val multiLine = commands.mkString(" \n ;", " \n ;", " \n ")
val start = System.nanoTime
assert(multiLine.parse == commands)
val elapsed = System.nanoTime - start
// Make sure this took less than 10 seconds. It takes about 30 milliseconds to run with
// 100 commands and 3 milliseconds with 3 commands. With a bad parser, it will run indefinitely.
assert(elapsed.nanoseconds < 10.seconds)
}
}