Instrument: Replace bindings object with Map [perf]

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;

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;
 }
 
 /**

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}'`);
 	}
 }

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

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;
 }

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,28 +568,28 @@ 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.
 	// NB: `hoistBlock.parent` here is either parent function params block or file block.
 	// 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.

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

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;
 	}

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;
 }