Skip to content

Commit b3b6bb4

Browse files
Gabriella439mergify[bot]
authored andcommitted
Partially fix whitespace parsing performance regression (#1512)
* Partially fix whitespace parsing performance regression This undoes some of the performance regression introduced in #1483 Before #1483: ``` benchmarked Line comment time 11.86 ms (11.69 ms .. 11.98 ms) 0.999 R² (0.999 R² .. 1.000 R²) mean 11.84 ms (11.79 ms .. 11.89 ms) std dev 129.4 μs (107.2 μs .. 164.1 μs) benchmarked Block comment time 13.20 ms (13.00 ms .. 13.41 ms) 0.999 R² (0.998 R² .. 1.000 R²) mean 13.59 ms (13.41 ms .. 13.94 ms) std dev 600.0 μs (142.2 μs .. 953.7 μs) ``` After #1483: ``` benchmarked Line comment time 288.7 ms (282.8 ms .. 294.7 ms) 1.000 R² (0.999 R² .. 1.000 R²) mean 292.3 ms (290.8 ms .. 294.6 ms) std dev 3.156 ms (2.216 ms .. 4.546 ms) benchmarked Block comment time 286.2 ms (280.9 ms .. 292.6 ms) 0.999 R² (0.998 R² .. 1.000 R²) mean 290.6 ms (288.3 ms .. 292.9 ms) std dev 3.875 ms (2.866 ms .. 5.500 ms) ``` After this change: ``` benchmarked Line comment time 61.44 ms (60.37 ms .. 63.03 ms) 0.999 R² (0.997 R² .. 1.000 R²) mean 61.41 ms (60.74 ms .. 62.25 ms) std dev 1.341 ms (945.0 μs .. 1.901 ms) benchmarked Block comment time 61.83 ms (60.97 ms .. 63.14 ms) 0.999 R² (0.998 R² .. 1.000 R²) mean 61.16 ms (60.33 ms .. 61.85 ms) std dev 1.396 ms (1.011 ms .. 1.907 ms) ``` * Correctly parse `https://example.com usingBla` ... as caught by @sjakobi
1 parent 7eec31d commit b3b6bb4

File tree

2 files changed

+63
-20
lines changed

2 files changed

+63
-20
lines changed

dhall/src/Dhall/Parser/Expression.hs

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -262,14 +262,14 @@ parsers embedded = Parsers {..}
262262
a <- operatorExpression
263263

264264
let alternative4A = do
265-
try (whitespace *> _arrow)
265+
_arrow
266266
whitespace
267267
b <- expression
268268
whitespace
269269
return (Pi "_" a b)
270270

271271
let alternative4B = do
272-
try (whitespace *> _colon)
272+
_colon
273273
nonemptyWhitespace
274274
b <- expression
275275
case shallowDenote a of
@@ -289,9 +289,16 @@ parsers embedded = Parsers {..}
289289
makeOperatorExpression operatorParser subExpression =
290290
noted (do
291291
a <- subExpression
292+
293+
whitespace
294+
292295
b <- Text.Megaparsec.many $ do
293-
op <- try (whitespace *> operatorParser)
296+
op <- operatorParser
297+
294298
r <- subExpression
299+
300+
whitespace
301+
295302
return (\l -> l `op` r)
296303
return (foldl (\x f -> f x) a b) )
297304

@@ -705,36 +712,70 @@ parsers embedded = Parsers {..}
705712
nonEmptyRecordTypeOrLiteral = do
706713
a <- anyLabel
707714

715+
whitespace
716+
708717
let nonEmptyRecordType = do
709-
try (whitespace *> _colon)
718+
_colon
719+
710720
nonemptyWhitespace
721+
711722
b <- expression
723+
724+
whitespace
725+
712726
e <- Text.Megaparsec.many (do
713-
try (whitespace *> _comma)
727+
_comma
728+
714729
whitespace
730+
715731
c <- anyLabel
732+
716733
whitespace
734+
717735
_colon
736+
718737
nonemptyWhitespace
738+
719739
d <- expression
740+
741+
whitespace
742+
720743
return (c, d) )
744+
721745
m <- toMap ((a, b) : e)
746+
722747
return (Record m)
723748

724749
let nonEmptyRecordLiteral = do
725-
try (whitespace *> _equal)
750+
_equal
751+
726752
whitespace
753+
727754
b <- expression
755+
756+
whitespace
757+
728758
e <- Text.Megaparsec.many (do
729-
try (whitespace *> _comma)
759+
_comma
760+
730761
whitespace
762+
731763
c <- anyLabel
764+
732765
whitespace
766+
733767
_equal
768+
734769
whitespace
770+
735771
d <- expression
772+
773+
whitespace
774+
736775
return (c, d) )
776+
737777
m <- toMap ((a, b) : e)
778+
738779
return (RecordLit m)
739780

740781
nonEmptyRecordType <|> nonEmptyRecordLiteral
@@ -840,10 +881,8 @@ local = do
840881
http :: Parser ImportType
841882
http = do
842883
url <- httpRaw
843-
headers <- optional (try $ do
844-
whitespace
845-
_using
846-
nonemptyWhitespace
884+
headers <- optional (do
885+
try (whitespace *> _using *> nonemptyWhitespace)
847886
importExpression import_ )
848887
return (Remote (url { headers }))
849888

@@ -905,10 +944,9 @@ import_ = (do
905944
importMode <- alternative <|> pure Code
906945
return (Import {..}) ) <?> "import"
907946
where
908-
alternative = try $ do
909-
whitespace
910-
_as
911-
nonemptyWhitespace
947+
alternative = do
948+
try (whitespace *> _as *> nonemptyWhitespace)
949+
912950
(_Text >> pure RawText) <|> (_Location >> pure Location)
913951

914952
-- | Same as @Data.Text.splitOn@, except always returning a `NonEmpty` result

dhall/tests/Dhall/Test/Parser.hs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ notesInLetInLet = do
8989
(Just " ")
9090
Nothing
9191
(Just " ")
92-
(Note "0" (NaturalLit 0)))
92+
(Note "0 " (Note "0" (NaturalLit 0)))
93+
)
9394
-- This 'Let' isn't wrapped in a 'Note'!
9495
(Let
9596
(Binding
@@ -98,7 +99,7 @@ notesInLetInLet = do
9899
(Just " ")
99100
Nothing
100101
(Just " ")
101-
(Note "1" (NaturalLit 1))
102+
(Note "1 " (Note "1" (NaturalLit 1)))
102103
)
103104
(Note "let z = 2 in x"
104105
(Let
@@ -108,10 +109,14 @@ notesInLetInLet = do
108109
(Just " ")
109110
Nothing
110111
(Just " ")
111-
(Note "2" (NaturalLit 2))
112+
(Note "2 " (Note "2" (NaturalLit 2)))
112113
)
113-
(Note "x"
114-
(Var (V "x" 0))))))))
114+
(Note "x" (Var (V "x" 0)))
115+
)
116+
)
117+
)
118+
)
119+
)
115120

116121
let msg = "Unexpected parse result"
117122

0 commit comments

Comments
 (0)