Skip to content

Commit 675916c

Browse files
kdy1kdy1
authoredJun 12, 2024··
perf(es/minifier): Do not visit var init multiple times (#9039)
**Description:** I mistakenly introduced a performance regression with #9032. It makes the minifier visit the initializer of variables multiple times - once while normal visiting and once in `hoist_props_of_vars`. This PR fixes it.
·
v1.11.16swc_core@v0.106.1
1 parent 65bd215 commit 675916c

File tree

22 files changed

+323
-314
lines changed

22 files changed

+323
-314
lines changed
 

‎crates/swc_ecma_minifier/src/compress/optimize/mod.rs

+30-4
Original file line numberDiff line numberDiff line change
@@ -2955,10 +2955,38 @@ impl VisitMut for Optimizer<'_> {
29552955
return false;
29562956
}
29572957

2958-
var.visit_mut_with(self);
2958+
true
2959+
});
2960+
2961+
{
2962+
// We loop with index to avoid borrow checker issue.
2963+
// We use splice so we cannot use for _ in vars
2964+
let mut idx = 0;
2965+
2966+
while idx < vars.len() {
2967+
let var = &mut vars[idx];
2968+
var.visit_mut_with(self);
2969+
2970+
// The varaible is dropped.
2971+
if var.name.is_invalid() {
2972+
vars.remove(idx);
2973+
continue;
2974+
}
29592975

2976+
let new = self.hoist_props_of_var(var);
2977+
2978+
if let Some(new) = new {
2979+
let len = new.len();
2980+
vars.splice(idx..=idx, new);
2981+
idx += len;
2982+
} else {
2983+
idx += 1;
2984+
}
2985+
}
2986+
}
2987+
2988+
vars.retain_mut(|var| {
29602989
if var.name.is_invalid() {
2961-
// It will be inlined.
29622990
self.changed = true;
29632991
return false;
29642992
}
@@ -2968,8 +2996,6 @@ impl VisitMut for Optimizer<'_> {
29682996
true
29692997
});
29702998

2971-
self.hoist_props_of_vars(vars);
2972-
29732999
let uses_eval = self.data.scopes.get(&self.ctx.scope).unwrap().has_eval_call;
29743000

29753001
if !uses_eval && !self.ctx.dont_use_prepend_nor_append {

‎crates/swc_ecma_minifier/src/compress/optimize/props.rs

+7-24
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,29 @@
11
use swc_common::{util::take::Take, DUMMY_SP};
22
use swc_ecma_ast::*;
33
use swc_ecma_utils::{contains_this_expr, private_ident, prop_name_eq, ExprExt};
4-
use swc_ecma_visit::VisitMutWith;
54

65
use super::{unused::PropertyAccessOpts, Optimizer};
76
use crate::util::deeply_contains_this_expr;
87

98
/// Methods related to the option `hoist_props`.
109
impl Optimizer<'_> {
11-
pub(super) fn hoist_props_of_vars(&mut self, n: &mut Vec<VarDeclarator>) {
10+
pub(super) fn hoist_props_of_var(
11+
&mut self,
12+
n: &mut VarDeclarator,
13+
) -> Option<Vec<VarDeclarator>> {
1214
if !self.options.hoist_props {
1315
log_abort!("hoist_props: option is disabled");
14-
return;
16+
return None;
1517
}
1618
if self.ctx.is_exported {
1719
log_abort!("hoist_props: Exported variable is not hoisted");
18-
return;
20+
return None;
1921
}
2022
if self.ctx.in_top_level() && !self.options.top_level() {
2123
log_abort!("hoist_props: Top-level variable is not hoisted");
22-
return;
23-
}
24-
25-
let mut new = Vec::with_capacity(n.len());
26-
for mut n in n.take() {
27-
if let Some(init) = &mut n.init {
28-
init.visit_mut_with(self);
29-
}
30-
31-
let new_vars = self.hoist_props_of_var(&mut n);
32-
33-
if let Some(new_vars) = new_vars {
34-
new.extend(new_vars);
35-
} else {
36-
new.push(n);
37-
}
24+
return None;
3825
}
3926

40-
*n = new;
41-
}
42-
43-
fn hoist_props_of_var(&mut self, n: &mut VarDeclarator) -> Option<Vec<VarDeclarator>> {
4427
if let Pat::Ident(name) = &mut n.name {
4528
if name.id.span.ctxt == self.marks.top_level_ctxt
4629
&& self.options.top_retain.contains(&name.id.sym)

‎crates/swc_ecma_minifier/tests/benches-full/echarts.js

+15-15
Original file line numberDiff line numberDiff line change
@@ -9609,7 +9609,7 @@
96099609
], upstreamSignList = [];
96109610
assert(resultSourceList && upstreamSignList), this._setLocalSource(resultSourceList, upstreamSignList);
96119611
}, SourceManager.prototype._applyTransform = function(upMgrList) {
9612-
var source, encodeDefine, sourceList, datasetModel = this._sourceHost, transformOption = datasetModel.get('transform', !0), fromTransformResult = datasetModel.get('fromTransformResult', !0);
9612+
var encodeDefine, source, sourceList, datasetModel = this._sourceHost, transformOption = datasetModel.get('transform', !0), fromTransformResult = datasetModel.get('fromTransformResult', !0);
96139613
assert(null != fromTransformResult || null != transformOption), null != fromTransformResult && 1 !== upMgrList.length && doThrow('When using `fromTransformResult`, there should be only one upstream dataset');
96149614
var upSourceList = [], upstreamSignList = [];
96159615
return (each(upMgrList, function(upMgr) {
@@ -15070,10 +15070,10 @@
1507015070
return null == precision ? precision = getPrecisionSafe(data.value) || 0 : 'auto' === precision && (precision = this._intervalPrecision), addCommas(round(data.value, precision, !0));
1507115071
}, IntervalScale.prototype.niceTicks = function(splitNumber, minInterval, maxInterval) {
1507215072
splitNumber = splitNumber || 5;
15073-
var splitNumber1, result, span, interval, precision, niceTickExtent, extent = this._extent, span1 = extent[1] - extent[0];
15073+
var splitNumber1, result, span, interval, precision, extent = this._extent, span1 = extent[1] - extent[0];
1507415074
if (isFinite(span1)) {
1507515075
span1 < 0 && (span1 = -span1, extent.reverse());
15076-
var result1 = (splitNumber1 = splitNumber, result = {}, span = extent[1] - extent[0], interval = result.interval = nice(span / splitNumber1, !0), null != minInterval && interval < minInterval && (interval = result.interval = minInterval), null != maxInterval && interval > maxInterval && (interval = result.interval = maxInterval), precision = result.intervalPrecision = getPrecisionSafe(interval) + 2, isFinite((niceTickExtent = result.niceTickExtent = [
15076+
var niceTickExtent, result1 = (splitNumber1 = splitNumber, result = {}, span = extent[1] - extent[0], interval = result.interval = nice(span / splitNumber1, !0), null != minInterval && interval < minInterval && (interval = result.interval = minInterval), null != maxInterval && interval > maxInterval && (interval = result.interval = maxInterval), precision = result.intervalPrecision = getPrecisionSafe(interval) + 2, isFinite((niceTickExtent = result.niceTickExtent = [
1507715077
round(Math.ceil(extent[0] / interval) * interval, precision),
1507815078
round(Math.floor(extent[1] / interval) * interval, precision)
1507915079
])[0]) || (niceTickExtent[0] = extent[0]), isFinite(niceTickExtent[1]) || (niceTickExtent[1] = extent[1]), clamp(niceTickExtent, 0, extent), clamp(niceTickExtent, 1, extent), niceTickExtent[0] > niceTickExtent[1] && (niceTickExtent[0] = niceTickExtent[1]), result);
@@ -16083,7 +16083,7 @@
1608316083
return ('category' === this.type ? (result = makeCategoryLabelsActually(this, labelModel = this.getLabelModel()), !labelModel.get('show') || this.scale.isBlank() ? {
1608416084
labels: [],
1608516085
labelCategoryInterval: result.labelCategoryInterval
16086-
} : result) : (ticks = (axis = this).scale.getTicks(), labelFormatter = makeLabelFormatter(axis), {
16086+
} : result) : (axis = this, ticks = axis.scale.getTicks(), labelFormatter = makeLabelFormatter(axis), {
1608716087
labels: map(ticks, function(tick, idx) {
1608816088
return {
1608916089
formattedLabel: labelFormatter(tick, idx),
@@ -19281,13 +19281,13 @@
1928119281
this._ctx = null;
1928219282
for(var i = 0; i < points.length;){
1928319283
var x = points[i++], y = points[i++];
19284-
isNaN(x) || isNaN(y) || this.softClipShape && !this.softClipShape.contain(x, y) || (symbolProxyShape.x = x - size[0] / 2, symbolProxyShape.y = y - size[1] / 2, symbolProxyShape.width = size[0], symbolProxyShape.height = size[1], symbolProxy.buildPath(path, symbolProxyShape, !0));
19284+
!(isNaN(x) || isNaN(y)) && (!this.softClipShape || this.softClipShape.contain(x, y)) && (symbolProxyShape.x = x - size[0] / 2, symbolProxyShape.y = y - size[1] / 2, symbolProxyShape.width = size[0], symbolProxyShape.height = size[1], symbolProxy.buildPath(path, symbolProxyShape, !0));
1928519285
}
1928619286
}, LargeSymbolPath.prototype.afterBrush = function() {
1928719287
var shape = this.shape, points = shape.points, size = shape.size, ctx = this._ctx;
1928819288
if (ctx) for(var i = 0; i < points.length;){
1928919289
var x = points[i++], y = points[i++];
19290-
isNaN(x) || isNaN(y) || this.softClipShape && !this.softClipShape.contain(x, y) || ctx.fillRect(x - size[0] / 2, y - size[1] / 2, size[0], size[1]);
19290+
!(isNaN(x) || isNaN(y)) && (!this.softClipShape || this.softClipShape.contain(x, y)) && ctx.fillRect(x - size[0] / 2, y - size[1] / 2, size[0], size[1]);
1929119291
}
1929219292
}, LargeSymbolPath.prototype.findDataIndex = function(x, y) {
1929319293
for(var shape = this.shape, points = shape.points, size = shape.size, w = Math.max(size[0], 4), h = Math.max(size[1], 4), idx = points.length / 2 - 1; idx >= 0; idx--){
@@ -20188,11 +20188,11 @@
2018820188
axisName: function(opt, axisModel, group, transformGroup) {
2018920189
var labelLayout, axisNameAvailableWidth, name = retrieve(opt.axisName, axisModel.get('name'));
2019020190
if (name) {
20191-
var textAlign, textVerticalAlign, rotationDiff, inverse, onLeft, nameLocation = axisModel.get('nameLocation'), nameDirection = opt.nameDirection, textStyleModel = axisModel.getModel('nameTextStyle'), gap = axisModel.get('nameGap') || 0, extent = axisModel.axis.getExtent(), gapSignal = extent[0] > extent[1] ? -1 : 1, pos = [
20191+
var rotation, textAlign, textVerticalAlign, rotationDiff, inverse, onLeft, nameLocation = axisModel.get('nameLocation'), nameDirection = opt.nameDirection, textStyleModel = axisModel.getModel('nameTextStyle'), gap = axisModel.get('nameGap') || 0, extent = axisModel.axis.getExtent(), gapSignal = extent[0] > extent[1] ? -1 : 1, pos = [
2019220192
'start' === nameLocation ? extent[0] - gapSignal * gap : 'end' === nameLocation ? extent[1] + gapSignal * gap : (extent[0] + extent[1]) / 2,
2019320193
isNameLocationCenter(nameLocation) ? opt.labelOffset + nameDirection * gap : 0
2019420194
], nameRotation = axisModel.get('nameRotate');
20195-
null != nameRotation && (nameRotation = nameRotation * PI$5 / 180), isNameLocationCenter(nameLocation) ? labelLayout = AxisBuilder.innerTextLayout(opt.rotation, null != nameRotation ? nameRotation : opt.rotation, nameDirection) : (rotationDiff = remRadian((nameRotation || 0) - opt.rotation), inverse = extent[0] > extent[1], onLeft = 'start' === nameLocation && !inverse || 'start' !== nameLocation && inverse, isRadianAroundZero(rotationDiff - PI$5 / 2) ? (textVerticalAlign = onLeft ? 'bottom' : 'top', textAlign = 'center') : isRadianAroundZero(rotationDiff - 1.5 * PI$5) ? (textVerticalAlign = onLeft ? 'top' : 'bottom', textAlign = 'center') : (textVerticalAlign = 'middle', textAlign = rotationDiff < 1.5 * PI$5 && rotationDiff > PI$5 / 2 ? onLeft ? 'left' : 'right' : onLeft ? 'right' : 'left'), labelLayout = {
20195+
null != nameRotation && (nameRotation = nameRotation * PI$5 / 180), isNameLocationCenter(nameLocation) ? labelLayout = AxisBuilder.innerTextLayout(opt.rotation, null != nameRotation ? nameRotation : opt.rotation, nameDirection) : (rotation = opt.rotation, rotationDiff = remRadian((nameRotation || 0) - rotation), inverse = extent[0] > extent[1], onLeft = 'start' === nameLocation && !inverse || 'start' !== nameLocation && inverse, isRadianAroundZero(rotationDiff - PI$5 / 2) ? (textVerticalAlign = onLeft ? 'bottom' : 'top', textAlign = 'center') : isRadianAroundZero(rotationDiff - 1.5 * PI$5) ? (textVerticalAlign = onLeft ? 'top' : 'bottom', textAlign = 'center') : (textVerticalAlign = 'middle', textAlign = rotationDiff < 1.5 * PI$5 && rotationDiff > PI$5 / 2 ? onLeft ? 'left' : 'right' : onLeft ? 'right' : 'left'), labelLayout = {
2019620196
rotation: rotationDiff,
2019720197
textAlign: textAlign,
2019820198
textVerticalAlign: textVerticalAlign
@@ -22924,19 +22924,19 @@
2292422924
],
2292522925
[
2292622926
x + itemWidth,
22927-
0 + itemHeight
22927+
y + itemHeight
2292822928
],
2292922929
[
2293022930
head ? x : x - 5,
22931-
0 + itemHeight
22931+
y + itemHeight
2293222932
]
2293322933
];
2293422934
return tail || points.splice(2, 0, [
2293522935
x + itemWidth + 5,
22936-
0 + itemHeight / 2
22936+
y + itemHeight / 2
2293722937
]), head || points.push([
2293822938
x,
22939-
0 + itemHeight / 2
22939+
y + itemHeight / 2
2294022940
]), points;
2294122941
}(lastX, 0, itemWidth, height, i === renderList.length - 1, 0 === i)
2294222942
},
@@ -28681,7 +28681,7 @@
2868128681
]);
2868228682
}, LinesView.prototype._clearLayer = function(api) {
2868328683
var zr = api.getZr();
28684-
'svg' === zr.painter.getType() || null == this._lastZlevel || zr.painter.getLayer(this._lastZlevel).clear(!0);
28684+
'svg' !== zr.painter.getType() && null != this._lastZlevel && zr.painter.getLayer(this._lastZlevel).clear(!0);
2868528685
}, LinesView.prototype.remove = function(ecModel, api) {
2868628686
this._lineDraw && this._lineDraw.remove(), this._lineDraw = null, this._clearLayer(api);
2868728687
}, LinesView.type = 'lines', LinesView;
@@ -34689,7 +34689,7 @@
3468934689
return null !== _super && _super.apply(this, arguments) || this;
3469034690
}
3469134691
return __extends(DataView, _super), DataView.prototype.onclick = function(ecModel, api) {
34692-
var seriesGroupByCategoryAxis, otherSeries, meta, result, groups, tables, container = api.getDom(), model = this.model;
34692+
var seriesGroupByCategoryAxis, otherSeries, meta, groups, tables, result, container = api.getDom(), model = this.model;
3469334693
this._dom && container.removeChild(this._dom);
3469434694
var root = document.createElement('div');
3469534695
root.style.cssText = 'position:absolute;left:5px;top:5px;bottom:5px;right:5px;', root.style.backgroundColor = model.get('backgroundColor') || '#fff';
@@ -35481,7 +35481,7 @@
3548135481
}, TooltipHTMLContent.prototype.show = function(tooltipModel, nearPointColor) {
3548235482
clearTimeout(this._hideTimeout), clearTimeout(this._longHideTimeout);
3548335483
var enableTransition, onlyFade, cssText, transitionDuration, backgroundColor, shadowBlur, shadowColor, shadowOffsetX, shadowOffsetY, textStyleModel, padding, transitionCurve, transitionOption, transitionText, cssText1, fontSize, color, shadowColor1, shadowBlur1, shadowOffsetX1, shadowOffsetY1, el = this.el, style = el.style, styleCoord = this._styleCoord;
35484-
el.innerHTML ? style.cssText = gCssText + (enableTransition = !this._firstShow, onlyFade = this._longHide, cssText = [], transitionDuration = tooltipModel.get('transitionDuration'), backgroundColor = tooltipModel.get('backgroundColor'), shadowBlur = tooltipModel.get('shadowBlur'), shadowColor = tooltipModel.get('shadowColor'), shadowOffsetX = tooltipModel.get('shadowOffsetX'), shadowOffsetY = tooltipModel.get('shadowOffsetY'), textStyleModel = tooltipModel.getModel('textStyle'), padding = getPaddingFromTooltipModel(tooltipModel, 'html'), cssText.push('box-shadow:' + shadowOffsetX + "px " + shadowOffsetY + "px " + shadowBlur + "px " + shadowColor), enableTransition && transitionDuration && cssText.push((transitionText = "opacity" + (transitionOption = " " + transitionDuration / 2 + "s " + (transitionCurve = 'cubic-bezier(0.23,1,0.32,1)')) + ",visibility" + transitionOption, onlyFade || (transitionOption = " " + transitionDuration + "s " + transitionCurve, transitionText += env.transformSupported ? "," + TRANSFORM_VENDOR + transitionOption : ",left" + transitionOption + ",top" + transitionOption), CSS_TRANSITION_VENDOR + ':' + transitionText)), backgroundColor && (env.canvasSupported ? cssText.push('background-color:' + backgroundColor) : (cssText.push('background-color:#' + toHex(backgroundColor)), cssText.push('filter:alpha(opacity=70)'))), each([
35484+
el.innerHTML ? style.cssText = gCssText + (enableTransition = !this._firstShow, onlyFade = this._longHide, cssText = [], transitionDuration = tooltipModel.get('transitionDuration'), backgroundColor = tooltipModel.get('backgroundColor'), shadowBlur = tooltipModel.get('shadowBlur'), shadowColor = tooltipModel.get('shadowColor'), shadowOffsetX = tooltipModel.get('shadowOffsetX'), shadowOffsetY = tooltipModel.get('shadowOffsetY'), textStyleModel = tooltipModel.getModel('textStyle'), padding = getPaddingFromTooltipModel(tooltipModel, 'html'), cssText.push('box-shadow:' + (shadowOffsetX + "px " + shadowOffsetY + "px ") + shadowBlur + "px " + shadowColor), enableTransition && transitionDuration && cssText.push((transitionText = "opacity" + (transitionOption = " " + transitionDuration / 2 + "s " + (transitionCurve = 'cubic-bezier(0.23,1,0.32,1)')) + ",visibility" + transitionOption, onlyFade || (transitionOption = " " + transitionDuration + "s " + transitionCurve, transitionText += env.transformSupported ? "," + TRANSFORM_VENDOR + transitionOption : ",left" + transitionOption + ",top" + transitionOption), CSS_TRANSITION_VENDOR + ':' + transitionText)), backgroundColor && (env.canvasSupported ? cssText.push('background-color:' + backgroundColor) : (cssText.push('background-color:#' + toHex(backgroundColor)), cssText.push('filter:alpha(opacity=70)'))), each([
3548535485
'width',
3548635486
'color',
3548735487
'radius'

‎crates/swc_ecma_minifier/tests/benches-full/jquery.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,8 @@
248248
}
249249
function createDisabledPseudo(disabled) {
250250
return function(elem) {
251-
return "form" in elem ? elem.parentNode && !1 === elem.disabled ? "label" in elem ? "label" in elem.parentNode ? elem.parentNode.disabled === disabled : elem.disabled === disabled : elem.isDisabled === disabled || !disabled !== elem.isDisabled && inDisabledFieldset(elem) === disabled : elem.disabled === disabled : "label" in elem && elem.disabled === disabled;
251+
if ("form" in elem) return elem.parentNode && !1 === elem.disabled ? "label" in elem ? "label" in elem.parentNode ? elem.parentNode.disabled === disabled : elem.disabled === disabled : elem.isDisabled === disabled || !disabled !== elem.isDisabled && inDisabledFieldset(elem) === disabled : elem.disabled === disabled;
252+
return "label" in elem && elem.disabled === disabled;
252253
};
253254
}
254255
function createPositionalPseudo(fn) {

0 commit comments

Comments
 (0)
Please sign in to comment.