Skip to content

Commit 4c8740d

Browse files
committed
Minify: Simplify fix for performance issue
As pointed out by Github user @paul-rouse, the performance issue was not due to the unbalanced tree, but rather a recursion which was occurring more than once.
1 parent 708346f commit 4c8740d

File tree

1 file changed

+8
-28
lines changed

1 file changed

+8
-28
lines changed

src/Language/JavaScript/Process/Minify.hs

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ instance MinifyJS JSExpression where
145145
fix a (JSCallExpressionDot ex _ xs) = JSCallExpressionDot (fix a ex) emptyAnnot (fixEmpty xs)
146146
fix a (JSCallExpressionSquare ex _ xs _) = JSCallExpressionSquare (fix a ex) emptyAnnot (fixEmpty xs) emptyAnnot
147147
fix a (JSCommaExpression le _ re) = JSCommaExpression (fix a le) emptyAnnot (fixEmpty re)
148-
fix a (JSExpressionBinary lhs op rhs) = fixBalanceBinOpExpr a lhs op rhs
148+
fix a (JSExpressionBinary lhs op rhs) = fixBinOpExpression a op lhs rhs
149149
fix _ (JSExpressionParen _ e _) = JSExpressionParen emptyAnnot (fixEmpty e) emptyAnnot
150150
fix a (JSExpressionPostfix e op) = JSExpressionPostfix (fix a e) (fixEmpty op)
151151
fix a (JSExpressionTernary cond _ v1 _ v2) = JSExpressionTernary (fix a cond) emptyAnnot (fixEmpty v1) emptyAnnot (fixEmpty v2)
@@ -164,38 +164,18 @@ fixVarList (JSLCons h _ v) = JSLCons (fixVarList h) emptyAnnot (fixEmpty v)
164164
fixVarList (JSLOne a) = JSLOne (fixSpace a)
165165
fixVarList JSLNil = JSLNil
166166

167-
-- --------------------------------
168-
-- By default, the parser generates binary expression tree which for long strings of
169-
-- the same operator are unbalanced. This can cause terrible performance problems in
170-
-- (for instance) the minifier (eg 4^n). Therefore, as we build the AST, we rebalance
171-
-- the JSExpressionBinary parts of the tree on the fly.
172-
173-
fixBalanceBinOpExpr :: JSAnnot -> JSExpression -> JSBinOp -> JSExpression -> JSExpression
174-
fixBalanceBinOpExpr ann left binop right =
175-
let JSExpressionBinary l _ r = rebalance binop $ flatten binop left ++ flatten binop right
176-
in fixBinOpExpression ann binop (fixEmpty l) (fixEmpty r)
177-
where
178-
-- Flatten the expression tree into a list.
179-
flatten op expr@(JSExpressionBinary l opx r)
180-
| binOpEq op opx = flatten op l ++ flatten op r
181-
| otherwise = [expr]
182-
flatten _ expr = [expr]
183-
184-
-- Create a balance tree from a list.
185-
rebalance _ [] = error "fixBalanceBinOpExpr"
186-
rebalance _ [x] = x
187-
-- rebalance op [l, r] = JSExpressionBinary l op r
188-
rebalance op xs = JSExpressionBinary (rebalance op l) op (rebalance op r)
189-
where (l, r) = splitAt (length xs `div` 2) xs
190-
191-
192-
193167
fixBinOpExpression :: JSAnnot -> JSBinOp -> JSExpression -> JSExpression -> JSExpression
194-
fixBinOpExpression _ (JSBinOpPlus _) (JSStringLiteral _ s1) (JSStringLiteral _ s2) = stringLitConcat (normalizeToSQ s1) (normalizeToSQ s2)
168+
fixBinOpExpression a (JSBinOpPlus _) lhs rhs = fixBinOpPlus a lhs rhs
195169
fixBinOpExpression a (JSBinOpIn _) lhs rhs = JSExpressionBinary (fix a lhs) (JSBinOpIn spaceAnnot) (fix spaceAnnot rhs)
196170
fixBinOpExpression a (JSBinOpInstanceOf _) lhs rhs = JSExpressionBinary (fix a lhs) (JSBinOpInstanceOf spaceAnnot) (fix spaceAnnot rhs)
197171
fixBinOpExpression a op lhs rhs = JSExpressionBinary (fix a lhs) (fixEmpty op) (fixEmpty rhs)
198172

173+
fixBinOpPlus :: JSAnnot -> JSExpression -> JSExpression -> JSExpression
174+
fixBinOpPlus a lhs rhs =
175+
case (fix a lhs, fixEmpty rhs) of
176+
(JSStringLiteral _ s1, JSStringLiteral _ s2) -> stringLitConcat (normalizeToSQ s1) (normalizeToSQ s2)
177+
(nlhs, nrhs) -> JSExpressionBinary nlhs (JSBinOpPlus emptyAnnot) nrhs
178+
199179
-- Concatenate two JSStringLiterals. Since the strings will include the string
200180
-- terminators (either single or double quotes) we use whatever terminator is
201181
-- used by the first string.

0 commit comments

Comments
 (0)