From b6a657b76c5bce2b05f255b166c82ffabefb2af6 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Mon, 11 Dec 2023 01:08:26 +0000 Subject: [PATCH] Instrument: Replace `bindings` object with Map [perf] --- lib/init/eval.js | 2 +- lib/instrument/blocks.js | 6 ++++-- lib/instrument/visitors/assignee.js | 8 ++++---- lib/instrument/visitors/class.js | 2 +- lib/instrument/visitors/eval.js | 4 ++-- lib/instrument/visitors/function.js | 12 ++++++------ lib/instrument/visitors/identifier.js | 6 +++--- lib/instrument/visitors/loop.js | 2 +- lib/instrument/visitors/super.js | 8 +++++--- 9 files changed, 27 insertions(+), 23 deletions(-) diff --git a/lib/init/eval.js b/lib/init/eval.js index 7d133eec..ee1e9a0c 100644 --- a/lib/init/eval.js +++ b/lib/init/eval.js @@ -275,7 +275,7 @@ function compile( // Filter out any temp vars which aren't used in `eval`-ed code if (tempVars.length > 0) { tempVars = tempVars.filter((tempVar) => { - const varNode = tempVar.block.bindings[tempVar.varName]?.varNode; + const varNode = tempVar.block.bindings.get(tempVar.varName)?.varNode; if (!varNode) return false; params.push(varNode); return true; diff --git a/lib/instrument/blocks.js b/lib/instrument/blocks.js index 77aa1249..69aabf43 100644 --- a/lib/instrument/blocks.js +++ b/lib/instrument/blocks.js @@ -58,7 +58,7 @@ function createBlockWithId(id, name, isVarsBlock, state) { id, name, parent, - bindings: Object.create(null), + bindings: new Map(), // Keyed by binding name varsBlock: undefined, scopeIdVarNode: undefined, tempVarNodes: undefined @@ -129,7 +129,7 @@ function createBinding(block, varName, props, state) { * @returns {Object} - Binding object */ function createBindingWithoutNameCheck(block, varName, props) { - return block.bindings[varName] = { // eslint-disable-line no-return-assign + const binding = { name: varName, varNode: props.varNode, isConst: !!props.isConst, @@ -139,6 +139,8 @@ function createBindingWithoutNameCheck(block, varName, props) { argNames: props.argNames, trails: [] // Trails of usages within same function as binding, where var not frozen }; + block.bindings.set(varName, binding); + return binding; } /** diff --git a/lib/instrument/visitors/assignee.js b/lib/instrument/visitors/assignee.js index 54739b54..14433f4b 100644 --- a/lib/instrument/visitors/assignee.js +++ b/lib/instrument/visitors/assignee.js @@ -176,7 +176,7 @@ function IdentifierVar(node, state) { const varName = node.name, fn = state.currentFunction, block = state.currentHoistBlock; - let binding = block.bindings[varName]; + let binding = block.bindings.get(varName); if (!binding) { // If function has a param with same name, create a separate binding on the hoist block, // but reuse the same binding object. This is so that if the param is frozen, @@ -186,11 +186,11 @@ function IdentifierVar(node, state) { // function body. Could fix that by visiting function params first, then create `arguments` binding, // and then visiting function body. // https://github.com/overlookmotel/livepack/issues/547 - binding = block.parent.bindings[varName]; + binding = block.parent.bindings.get(varName); if (binding) { // Param binding with same name as var. // Flag as `isVar` to allow function declarations to reuse same binding. - block.bindings[varName] = binding; + block.bindings.set(varName, binding); binding.isVar = true; } else { binding = createBinding(block, varName, {isVar: true}, state); @@ -218,7 +218,7 @@ function IdentifierVar(node, state) { * @throws {Error} - If is a clashing CommonJS var declaration */ function assertNoCommonJsVarsClash(block, varName, state) { - if (block === state.programBlock && varName !== 'arguments' && state.fileBlock.bindings[varName]) { + if (block === state.programBlock && varName !== 'arguments' && state.fileBlock.bindings.has(varName)) { throw new Error(`Clashing binding for var '${varName}'`); } } diff --git a/lib/instrument/visitors/class.js b/lib/instrument/visitors/class.js index f558ce7c..ec15134f 100644 --- a/lib/instrument/visitors/class.js +++ b/lib/instrument/visitors/class.js @@ -276,7 +276,7 @@ function visitClass(classNode, parent, key, className, state) { const innerNameBlock = createBlockWithId(innerNameBlockId, className, false, state); constructorParamsBlock.parent = innerNameBlock; if (protoThisBlock) protoThisBlock.parent = innerNameBlock; - innerNameBlock.bindings[className] = nameBlock.bindings[className]; + innerNameBlock.bindings.set(className, nameBlock.bindings.get(className)); } // Exit class body diff --git a/lib/instrument/visitors/eval.js b/lib/instrument/visitors/eval.js index eaed5e7f..d9f634d0 100644 --- a/lib/instrument/visitors/eval.js +++ b/lib/instrument/visitors/eval.js @@ -134,7 +134,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP if (!isExternalToFunction && fn && block.id < fn.id) isExternalToFunction = true; const varDefsNodes = []; - for (const [varName, binding] of Object.entries(block.bindings)) { + for (const [varName, binding] of block.bindings) { // Freeze binding to prevent var being renamed when used internally in a function. // All vars in scope need to be frozen, even if not accessible to `eval()` // because if they're renamed, they would become accessible to `eval()` when they weren't before. @@ -251,7 +251,7 @@ function instrumentEvalIdentifier(node, parent, key, state) { */ function isGlobalEval(block) { do { - if (block.bindings.eval) return false; + if (block.bindings.has('eval')) return false; } while (block = block.parent); // eslint-disable-line no-cond-assign return true; } diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index 1793130f..c847aed8 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -84,7 +84,7 @@ function FunctionDeclaration(node, state, parent, key) { // `() => { console.log(typeof x); q: function x() {} }` -> Logs 'function' // `() => { console.log(typeof x); if (true) function x() {} }` -> Logs 'undefined' const {currentBlock: block, currentHoistBlock: hoistBlock} = state; - let binding = block.bindings[fnName]; + let binding = block.bindings.get(fnName); if (!binding) { const isTopLevel = block === hoistBlock; binding = createBinding(block, fnName, {isVar: isTopLevel, isFrozenName: true}, state); @@ -227,7 +227,7 @@ function visitFunctionOrMethod( // If is full function, create binding for `arguments` in params block. // Param called `arguments` takes precedence. - if (isFullFunction && !paramsBlock.bindings.arguments) { + if (isFullFunction && !paramsBlock.bindings.has('arguments')) { createArgumentsBinding(paramsBlock, isStrict, argNames); } @@ -568,7 +568,7 @@ function hoistSloppyFunctionDeclaration(declaration) { const {varName, hoistBlock} = declaration; // Do not hoist if existing const, let or class declaration binding in hoist block - const hoistBlockBinding = hoistBlock.bindings[varName]; + const hoistBlockBinding = hoistBlock.bindings.get(varName); if (hoistBlockBinding && !hoistBlockBinding.isVar) return; // If parent function's params include var with same name, do not hoist. @@ -576,20 +576,20 @@ function hoistSloppyFunctionDeclaration(declaration) { // In CommonJS code, file block is CommonJS wrapper function's params, and in any other context // file block contains no bindings except `this`. So it's safe to treat as a params block in all cases. // NB: `paramsBlockBinding.argNames` check is to avoid the pseudo-param `arguments` blocking hoisting. - const paramsBlockBinding = hoistBlock.parent.bindings[varName]; + const paramsBlockBinding = hoistBlock.parent.bindings.get(varName); if (paramsBlockBinding && !paramsBlockBinding.argNames) return; // If any binding in intermediate blocks, do not hoist. // NB: This includes function declarations which will be themselves hoisted. let inBetweenBlock = declaration.block.parent; while (inBetweenBlock !== hoistBlock) { - if (inBetweenBlock.bindings[varName]) return; + if (inBetweenBlock.bindings.has(varName)) return; inBetweenBlock = inBetweenBlock.parent; } // Can be hoisted if (!hoistBlockBinding) { - hoistBlock.bindings[varName] = {...declaration.binding, isVar: true}; + hoistBlock.bindings.set(varName, {...declaration.binding, isVar: true}); } else if (!hoistBlockBinding.isFrozenName) { // Existing binding is a `var` declaration. // Flag binding as frozen so `var` declarations' identifiers don't get renamed. diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index 18d4617c..ed979344 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -73,7 +73,7 @@ function ThisExpression(node, state) { // Ignore if `this` is global const block = state.currentThisBlock, - binding = block.bindings.this; + binding = block.bindings.get('this'); if (!binding) return; // Record as external var use in function. Ignore if internal to function. @@ -100,7 +100,7 @@ function NewTargetExpression(node, state) { const block = state.currentThisBlock; if (block.id < fn.id) { recordExternalVar( - block.bindings['new.target'], block, 'new.target', fn, [...state.trail], true, false, state + block.bindings.get('new.target'), block, 'new.target', fn, [...state.trail], true, false, state ); } } @@ -149,7 +149,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // Find binding let binding; do { - binding = block.bindings[varName]; + binding = block.bindings.get(varName); } while (!binding && (block = block.parent)); // eslint-disable-line no-cond-assign // Record if global var diff --git a/lib/instrument/visitors/loop.js b/lib/instrument/visitors/loop.js index 0e83ab3c..0f5d7221 100644 --- a/lib/instrument/visitors/loop.js +++ b/lib/instrument/visitors/loop.js @@ -104,7 +104,7 @@ function ForXStatement(node, state) { // Without this, serializing outer function would allow `x` in `typeof x` to be mangled so it wouldn't // be a TDZ violation any more. state.currentBlock = parentBlock; - if (Object.keys(initBlock.bindings).length !== 0) { + if (initBlock.bindings.size !== 0) { const rightBlock = createAndEnterBlock('for', false, state); rightBlock.bindings = initBlock.bindings; } diff --git a/lib/instrument/visitors/super.js b/lib/instrument/visitors/super.js index e3e476b2..3b2195af 100644 --- a/lib/instrument/visitors/super.js +++ b/lib/instrument/visitors/super.js @@ -48,7 +48,9 @@ function Super(node, state, parent, key) { if (!isSuperCall && fnType === 'ArrowFunctionExpression') { const thisBlock = state.currentThisBlock; activateBlock(thisBlock, state); - const thisExternalVar = getOrCreateExternalVar(fn, thisBlock, 'this', thisBlock.bindings.this); + const thisExternalVar = getOrCreateExternalVar( + fn, thisBlock, 'this', thisBlock.bindings.get('this') + ); thisExternalVar.isReadFrom = true; } @@ -69,7 +71,7 @@ function Super(node, state, parent, key) { */ function activateSuperBinding(superBlock, state) { activateBlock(superBlock, state); - let binding = superBlock.bindings.super; + let binding = superBlock.bindings.get('super'); if (!binding) { // No need to add this binding to function, as `super` is always an external var. // `isFrozenName: true` because it shouldn't be added to internal vars if a function @@ -125,5 +127,5 @@ function recordAmendmentForSuper(node, superBlock, isSuperCall, fn, trail) { * @returns {Object|undefined} - Temp var identifier AST node, or `undefined` if binding not used */ function getSuperVarNode(superBlock) { - return superBlock.bindings.super?.varNode; + return superBlock.bindings.get('super')?.varNode; }