Skip to content

Commit

Permalink
perf(builtins): remove path.get calls (#857)
Browse files Browse the repository at this point in the history
  • Loading branch information
@vigneshshanmugam @boopathi
vigneshshanmugam authored and boopathi committed May 17, 2018
1 parent 9e447f6 commit 830ce3f
Showing 1 changed file with 67 additions and 65 deletions.
132 changes: 67 additions & 65 deletions packages/babel-plugin-minify-builtins/src/index.js
View file
Expand Up @@ -25,7 +25,7 @@ module.exports = function({ types: t }) {

const collectVisitor = {
AssignmentExpression(path) {
const left = path.get("left");
const { left } = path.node;

// Should bail and not run the plugin
// when builtin is polyfilled
Expand All @@ -42,12 +42,14 @@ module.exports = function({ types: t }) {
return;
}

const { node } = path;

if (
!isComputed(path) &&
isBuiltin(path) &&
!isComputed(node) &&
isBuiltin(node) &&
!getFunctionParent(path).isProgram()
) {
const expName = memberToString(path.node);
const expName = memberToString(node);
addToMap(context.pathsToUpdate, expName, path);
}
},
Expand All @@ -59,17 +61,19 @@ module.exports = function({ types: t }) {
return;
}

const { node } = callee;

// computed property should not be optimized
// Math[max]() -> Math.max()
if (!isComputed(callee) && isBuiltin(callee)) {
if (!isComputed(node) && isBuiltin(node)) {
const result = evaluate(path, { tdz: context.tdz });
// deopt when we have side effecty evaluate-able arguments
// Math.max(foo(), 1) --> untouched
// Math.floor(1) --> 1
if (result.confident && hasPureArgs(path)) {
path.replaceWith(t.valueToNode(result.value));
} else if (!getFunctionParent(callee).isProgram()) {
const expName = memberToString(callee.node);
const expName = memberToString(node);
addToMap(context.pathsToUpdate, expName, callee);
}
}
Expand Down Expand Up @@ -117,8 +121,8 @@ module.exports = function({ types: t }) {
}
};

function memberToString(memberExpr) {
const { object, property } = memberExpr;
function memberToString(memberExprNode) {
const { object, property } = memberExprNode;
let result = "";

if (t.isIdentifier(object)) result += object.name;
Expand All @@ -128,9 +132,8 @@ module.exports = function({ types: t }) {
return result;
}

function isBuiltInComputed(memberExpr) {
const { node } = memberExpr;
const { object, computed } = node;
function isBuiltInComputed(memberExprNode) {
const { object, computed } = memberExprNode;

return (
computed &&
Expand All @@ -139,8 +142,8 @@ module.exports = function({ types: t }) {
);
}

function isBuiltin(memberExpr) {
const { object, property } = memberExpr.node;
function isBuiltin(memberExprNode) {
const { object, property } = memberExprNode;

if (
t.isIdentifier(object) &&
Expand All @@ -152,63 +155,63 @@ module.exports = function({ types: t }) {
}
return false;
}
};

function addToMap(map, key, value) {
if (!map.has(key)) {
map.set(key, []);
}
map.get(key).push(value);
}

// Creates a segmented map that contains the earliest common Ancestor
// as the key and array of subpaths that are descendats of the LCA as value
function getSegmentedSubPaths(paths) {
let segments = new Map();

// Get earliest Path in tree where paths intersect
paths[0].getDeepestCommonAncestorFrom(
paths,
(lastCommon, index, ancestries) => {
// found the LCA
if (!lastCommon.isProgram()) {
let fnParent;
if (
lastCommon.isFunction() &&
lastCommon.get("body").isBlockStatement()
) {
segments.set(lastCommon, paths);
return;
} else if (
!(fnParent = getFunctionParent(lastCommon)).isProgram() &&
fnParent.get("body").isBlockStatement()
) {
segments.set(fnParent, paths);
return;
// Creates a segmented map that contains the earliest common Ancestor
// as the key and array of subpaths that are descendats of the LCA as value
function getSegmentedSubPaths(paths) {
let segments = new Map();

// Get earliest Path in tree where paths intersect
paths[0].getDeepestCommonAncestorFrom(
paths,
(lastCommon, index, ancestries) => {
// found the LCA
if (!lastCommon.isProgram()) {
let fnParent;
if (
lastCommon.isFunction() &&
t.isBlockStatement(lastCommon.node.body)
) {
segments.set(lastCommon, paths);
return;
} else if (
!(fnParent = getFunctionParent(lastCommon)).isProgram() &&

This comment has been minimized.

Sign in to view

Copy link
@DanielKoehler

DanielKoehler Jun 11, 2018

I've really not dug into this as might as I maybe should have, but in the last couple of months Meteor's standard-minifier-js plugin has started bubbling a "Cannot read property 'isProgram' of null while minifying app/app.js" -- from this line.

Is there a reason lastCommon.getFunctionParent() returning null isn't handled for here? Also, is there also some nice way I can trace lastCommon or the stack trace back to my ES6 source?

( meteor/meteor#9741 )
t.isBlockStatement(fnParent.node.body)
) {
segments.set(fnParent, paths);
return;
}
}
}
// Deopt and construct segments otherwise
for (const ancestor of ancestries) {
const fnPath = getChildFuncion(ancestor);
if (fnPath === void 0) {
continue;
// Deopt and construct segments otherwise
for (const ancestor of ancestries) {
const fnPath = getChildFuncion(ancestor);
if (fnPath === void 0) {
continue;
}
const validDescendants = paths.filter(p => {
return p.isDescendant(fnPath);
});
segments.set(fnPath, validDescendants);
}
const validDescendants = paths.filter(p => {
return p.isDescendant(fnPath);
});
segments.set(fnPath, validDescendants);
}
}
);
return segments;
}
);
return segments;
}

function getChildFuncion(ancestors = []) {
for (const path of ancestors) {
if (path.isFunction() && path.get("body").isBlockStatement()) {
return path;
function getChildFuncion(ancestors = []) {
for (const path of ancestors) {
if (path.isFunction() && t.isBlockStatement(path.node.body)) {
return path;
}
}
}
};

function addToMap(map, key, value) {
if (!map.has(key)) {
map.set(key, []);
}
map.get(key).push(value);
}

function hasPureArgs(path) {
Expand All @@ -221,8 +224,7 @@ function hasPureArgs(path) {
return true;
}

function isComputed(path) {
const { node } = path;
function isComputed(node) {
return node.computed;
}

Expand Down

0 comments on commit 830ce3f

Please sign in to comment.