From 242bc6928faab1305be3a420860a7f4a07348889 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Thu, 17 Nov 2016 08:48:26 -0800 Subject: [PATCH] Use array lookup for localized dependency IDs Summary: The current transform for require calls replaces strings with module-local IDs. That means that each module would need a local require function. To save hundreds of closure allocations, we can just use an array that maps local IDs to global IDs. This diff changes the dependency collection and replacement transform to change a call like `require('React')` to something like `require(_dependencyMap[0])` rather than `require(0)`. Reviewed By: cpojer Differential Revision: D4153714 fbshipit-source-id: a63455834c6c2a75da6977cacb9aac9f2cb1b3aa --- .../worker/__tests__/inline-test.js | 4 +- .../worker/collect-dependencies.js | 68 ++++++++++++++----- .../src/JSTransformer/worker/inline.js | 4 +- .../react-packager/src/ModuleGraph/worker.js | 16 +++-- 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/packager/react-packager/src/JSTransformer/worker/__tests__/inline-test.js b/packager/react-packager/src/JSTransformer/worker/__tests__/inline-test.js index 2278a157f7a..c97b7a974d0 100644 --- a/packager/react-packager/src/JSTransformer/worker/__tests__/inline-test.js +++ b/packager/react-packager/src/JSTransformer/worker/__tests__/inline-test.js @@ -295,8 +295,8 @@ describe('inline constants', () => { }); it('can work with transformed require calls', () => { - const code = `__arbitrary(function() { - var a = require(123, 'react-native').Platform.OS; + const code = `__arbitrary(require, function(arbitraryMapName) { + var a = require(arbitraryMapName[123], 'react-native').Platform.OS; });`; const {ast} = inline( 'arbitrary', {code}, {dev: true, platform: 'android', isWrapped: true}); diff --git a/packager/react-packager/src/JSTransformer/worker/collect-dependencies.js b/packager/react-packager/src/JSTransformer/worker/collect-dependencies.js index 1ae9d1d8165..7c662cd0f87 100644 --- a/packager/react-packager/src/JSTransformer/worker/collect-dependencies.js +++ b/packager/react-packager/src/JSTransformer/worker/collect-dependencies.js @@ -25,7 +25,8 @@ class Replacement { ); } - getIndex(name) { + getIndex(stringLiteral) { + const name = stringLiteral.value; let index = this.nameToIndex.get(name); if (index !== undefined) { return index; @@ -39,8 +40,9 @@ class Replacement { return Array.from(this.nameToIndex.keys()); } - makeArgs(newId, oldId) { - return [newId, oldId]; + makeArgs(newId, oldId, dependencyMapIdentifier) { + const mapLookup = createMapLookup(dependencyMapIdentifier, newId); + return [mapLookup, oldId]; } } @@ -52,14 +54,19 @@ class ProdReplacement { isRequireCall(callee, firstArg) { return ( - callee.type === 'Identifier' && callee.name === 'require' && - firstArg && firstArg.type === 'NumericLiteral' + callee.type === 'Identifier' && + callee.name === 'require' && + firstArg && + firstArg.type === 'MemberExpression' && + firstArg.property && + firstArg.property.type === 'NumericLiteral' ); } - getIndex(id) { + getIndex(memberExpression) { + const id = memberExpression.property.value; if (id in this.names) { - return this.replacement.getIndex(this.names[id]); + return this.replacement.getIndex({value: this.names[id]}); } throw new Error( @@ -72,27 +79,54 @@ class ProdReplacement { return this.replacement.getNames(); } - makeArgs(newId) { - return [newId]; + makeArgs(newId, _, dependencyMapIdentifier) { + const mapLookup = createMapLookup(dependencyMapIdentifier, newId); + return [mapLookup]; } } -function collectDependencies(ast, replacement) { +function createMapLookup(dependencyMapIdentifier, propertyIdentifier) { + return types.memberExpression( + dependencyMapIdentifier, + propertyIdentifier, + true, + ); +} + +function collectDependencies(ast, replacement, dependencyMapIdentifier) { + const traversalState = {dependencyMapIdentifier}; traverse(ast, { - CallExpression(path) { + Program(path, state) { + if (!state.dependencyMapIdentifier) { + state.dependencyMapIdentifier = + path.scope.generateUidIdentifier('dependencyMap'); + } + }, + CallExpression(path, state) { const node = path.node; const arg = node.arguments[0]; if (replacement.isRequireCall(node.callee, arg)) { - const index = replacement.getIndex(arg.value); - node.arguments = replacement.makeArgs(types.numericLiteral(index), arg); + const index = replacement.getIndex(arg); + node.arguments = replacement.makeArgs( + types.numericLiteral(index), + arg, + state.dependencyMapIdentifier, + ); } - } - }); + }, + }, null, traversalState); - return replacement.getNames(); + return { + dependencies: replacement.getNames(), + dependencyMapName: traversalState.dependencyMapIdentifier.name, + }; } exports = module.exports = ast => collectDependencies(ast, new Replacement()); exports.forOptimization = - (ast, names) => collectDependencies(ast, new ProdReplacement(names)); + (ast, names, dependencyMapName) => collectDependencies( + ast, + new ProdReplacement(names), + dependencyMapName && types.identifier(dependencyMapName), + ); diff --git a/packager/react-packager/src/JSTransformer/worker/inline.js b/packager/react-packager/src/JSTransformer/worker/inline.js index ee01821c038..5636e8e4f73 100644 --- a/packager/react-packager/src/JSTransformer/worker/inline.js +++ b/packager/react-packager/src/JSTransformer/worker/inline.js @@ -148,7 +148,9 @@ const plugin = () => inlinePlugin; function checkRequireArgs(args, dependencyId) { const pattern = t.stringLiteral(dependencyId); return t.isStringLiteral(args[0], pattern) || - t.isNumericLiteral(args[0]) && t.isStringLiteral(args[1], pattern); + t.isMemberExpression(args[0]) && + t.isNumericLiteral(args[0].property) && + t.isStringLiteral(args[1], pattern); } type AstResult = { diff --git a/packager/react-packager/src/ModuleGraph/worker.js b/packager/react-packager/src/ModuleGraph/worker.js index d034d892b50..c899743be73 100644 --- a/packager/react-packager/src/ModuleGraph/worker.js +++ b/packager/react-packager/src/ModuleGraph/worker.js @@ -144,16 +144,21 @@ function optimizeModule( } function makeResult(ast, filename, sourceCode, isPolyfill = false) { - const dependencies = isPolyfill ? [] : collectDependencies(ast); - const file = isPolyfill ? wrapPolyfill(ast) : wrapModule(ast); + const {dependencies, dependencyMapName} = isPolyfill + ? {dependencies: []} + : collectDependencies(ast); + const file = isPolyfill + ? wrapPolyfill(ast) + : wrapModule(ast, dependencyMapName); const gen = generate(file, filename, sourceCode); - return {code: gen.code, map: gen.map, dependencies}; + return {code: gen.code, map: gen.map, dependencies, dependencyMapName}; } -function wrapModule(file) { +function wrapModule(file, dependencyMapName) { const t = babel.types; - const factory = functionFromProgram(file.program, moduleFactoryParameters); + const params = moduleFactoryParameters.concat(dependencyMapName); + const factory = functionFromProgram(file.program, params); const def = t.callExpression(t.identifier('__d'), [factory]); return t.file(t.program([t.expressionStatement(def)])); } @@ -183,6 +188,7 @@ function optimize(transformed, file, originalCode, options) { : collectDependencies.forOptimization( optimized.ast, transformed.dependencies, + transformed.dependencyMapName, ); const inputMap = transformed.map;