From 021e9f767d35a6cf08bef8d95dc4e01ff7dd4480 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 3 Dec 2014 12:37:01 -0800 Subject: [PATCH] Don't include line number comments in isopackets The extra legibility is not as useful for code that is always part of core as it is for user's packages, and it actually has a notable (25%) effect on isopacket load time. Fixes #3218. --- tools/compiler.js | 10 ++++++++-- tools/isopack-cache.js | 5 ++++- tools/isopackets.js | 8 +++++++- tools/linker.js | 14 +++++++++++--- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/tools/compiler.js b/tools/compiler.js index c9266520e8..a498251334 100644 --- a/tools/compiler.js +++ b/tools/compiler.js @@ -45,6 +45,9 @@ compiler.compile = function (packageSource, options) { "` in package `" + packageSource.name + "`", rootPath: packageSource.sourceRoot }, function () { + // XXX we should probably also pass options.noLineNumbers into + // buildJsImage so it can pass it back to its call to + // compiler.compile var buildResult = bundler.buildJsImage({ name: info.name, packageMap: packageMap, @@ -125,7 +128,8 @@ compiler.compile = function (packageSource, options) { sourceArch: unibuild, isopackCache: isopackCache, nodeModulesPath: nodeModulesPath, - isPortable: isPortable + isPortable: isPortable, + noLineNumbers: options.noLineNumbers }); _.extend(pluginProviderPackageNames, unibuildResult.pluginProviderPackageNames); @@ -150,6 +154,7 @@ var compileUnibuild = function (options) { var isopackCache = options.isopackCache; var nodeModulesPath = options.nodeModulesPath; var isPortable = options.isPortable; + var noLineNumbers = options.noLineNumbers; var isApp = ! inputSourceArch.pkg.name; var resources = []; @@ -738,7 +743,8 @@ var compileUnibuild = function (options) { (inputSourceArch.kind === "main" ? "" : (":" + inputSourceArch.kind)) + ".js", name: inputSourceArch.pkg.name || null, declaredExports: _.pluck(inputSourceArch.declaredExports, 'name'), - jsAnalyze: jsAnalyze + jsAnalyze: jsAnalyze, + noLineNumbers: noLineNumbers }); // *** Determine captured variables diff --git a/tools/isopack-cache.js b/tools/isopack-cache.js index ad60f9f19d..eba846f071 100644 --- a/tools/isopack-cache.js +++ b/tools/isopack-cache.js @@ -36,6 +36,8 @@ exports.IsopackCache = function (options) { // tropohouse, and otherwise is a PackageMap object listing self._isopacks = {}; + self._noLineNumbers = !! options.noLineNumbers; + self.allLoadedLocalPackagesWatchSet = new watch.WatchSet; }; @@ -204,7 +206,8 @@ _.extend(exports.IsopackCache.prototype, { // Nope! Compile it again. var compilerResult = compiler.compile(packageInfo.packageSource, { packageMap: self._packageMap, - isopackCache: self + isopackCache: self, + noLineNumbers: self._noLineNumbers }); // Accept the compiler's result, even if there were errors (since it at // least will have a useful WatchSet and will allow us to keep going and diff --git a/tools/isopackets.js b/tools/isopackets.js index 719317be68..1d24792573 100644 --- a/tools/isopackets.js +++ b/tools/isopackets.js @@ -236,7 +236,13 @@ var makeIsopacketBuildContext = function () { // Make an isopack cache that doesn't save isopacks to disk and has no // access to versioned packages. context.isopackCache = new isopackCacheModule.IsopackCache({ - packageMap: context.packageMap + packageMap: context.packageMap, + // When linking JS files, don't include the padding spaces and line number + // comments. Since isopackets are loaded as part of possibly very short + // 'meteor' tool command invocations, we care more about startup time than + // legibility, and the difference is actually observable (eg 25% speedup + // loading constraint-solver). + noLineNumbers: true }); return context; }; diff --git a/tools/linker.js b/tools/linker.js index eb895f147e..2923fd1ba0 100644 --- a/tools/linker.js +++ b/tools/linker.js @@ -32,6 +32,7 @@ var Module = function (options) { self.combinedServePath = options.combinedServePath; self.importStubServePath = options.importStubServePath; self.jsAnalyze = options.jsAnalyze; + self.noLineNumbers = options.noLineNumbers; }; _.extend(Module.prototype, { @@ -134,7 +135,10 @@ _.extend(Module.prototype, { _.each(self.files, function (file) { if (!_.isEmpty(chunks)) chunks.push("\n\n\n\n\n\n"); - chunks.push(file.getPrelinkedOutput({ sourceWidth: sourceWidth })); + chunks.push(file.getPrelinkedOutput({ + sourceWidth: sourceWidth, + noLineNumbers: self.noLineNumbers + })); }); var node = new sourcemap.SourceNode(null, null, null, chunks); @@ -291,6 +295,8 @@ _.extend(File.prototype, { // - preserveLineNumbers: if true, decorate minimally so that line // numbers don't change between input and output. In this case, // sourceWidth is ignored. + // - noLineNumbers: We still include the banners and such, but + // no line number suffix. // - sourceWidth: width in columns to use for the source code // // Returns a SourceNode. @@ -355,7 +361,8 @@ _.extend(File.prototype, { _.each(lines, function (line) { var suffix = "\n"; - if (line.length <= width && line[line.length - 1] !== "\\") { + if (! options.noLineNumbers + && line.length <= width && line[line.length - 1] !== "\\") { suffix = padding.slice(line.length, width) + " // " + num + "\n"; } f(line, suffix, num); @@ -494,7 +501,8 @@ var prelink = function (options) { useGlobalNamespace: options.useGlobalNamespace, importStubServePath: options.importStubServePath, combinedServePath: options.combinedServePath, - jsAnalyze: options.jsAnalyze + jsAnalyze: options.jsAnalyze, + noLineNumbers: options.noLineNumbers }); _.each(options.inputFiles, function (inputFile) {