From 79d643574a1f97fbc19d5c6c914066ec10d4a538 Mon Sep 17 00:00:00 2001 From: zodern Date: Tue, 5 Feb 2019 12:10:51 -0600 Subject: [PATCH 1/6] Skip calculating SRI for assets from the public/ folder. Saves on average ~2ms per file in one app, which adds up when there are hundreds. SRI is currently only supported for js and css files, and Meteor only uses it for the main bundles. --- tools/isobuild/bundler.js | 11 +++++++++-- tools/isobuild/compiler.js | 7 ++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 3c097a5391..60cb232e8e 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -546,6 +546,9 @@ class File { // disk). this.sourcePath = options.sourcePath; + // Allows not calculating sri when the file doesn't support it + this._skipSri = options.skipSri + // info is just for help with debugging the tool; it isn't written to disk or // anything. this.info = options.info || '?'; @@ -619,7 +622,9 @@ class File { hashes.push(this._inputHash); } - hashes.push(this.sri()); + if (!this._skipSri) { + hashes.push(this.sri()); + } this._hash = watch.sha1(...hashes); } @@ -628,7 +633,7 @@ class File { } sri() { - if (! this._sri) { + if (! this._sri && !this._skipSri) { this._sri = watch.sha512(this.contents()); } @@ -1160,6 +1165,8 @@ class Target { data: resource.data, cacheable: false, hash: resource.hash, + sourcePath: resource.sourcePath, + skipSri: !!resource.sourcePath && resource.hash }; const file = new File(fileOptions); diff --git a/tools/isobuild/compiler.js b/tools/isobuild/compiler.js index 4f8c5a2ca3..91118ba9ab 100644 --- a/tools/isobuild/compiler.js +++ b/tools/isobuild/compiler.js @@ -457,7 +457,7 @@ var compileUnibuild = Profile(function (options) { // This function needs to be factored out to support legacy handlers later on // in the compilation process - function addAsset(contents, relPath, hash) { + function addAsset(contents, relPath, absPath, hash) { // XXX hack to strip out private and public directory names from app asset // paths if (! inputSourceArch.pkg.name) { @@ -470,7 +470,8 @@ var compileUnibuild = Profile(function (options) { path: relPath, servePath: colonConverter.convert( files.pathJoin(inputSourceArch.pkg.serveRoot, relPath)), - hash: hash + hash: hash, + sourcePath: absPath }); } @@ -482,7 +483,7 @@ var compileUnibuild = Profile(function (options) { const hash = optimisticHashOrNull(absPath) const contents = optimisticReadFile(absPath) - addAsset(contents, relPath, hash); + addAsset(contents, relPath, absPath, hash); }); // Add and compile all source files From b48715d804101e343d87f87e3a839216009e74eb Mon Sep 17 00:00:00 2001 From: zodern Date: Tue, 5 Feb 2019 12:39:58 -0600 Subject: [PATCH 2/6] Improve performance of minifying dynamic files. Avoids converting file contents to and from buffers and strings. The conversion had been done for dynamic files before minifying them, and all files after they were minified. --- tools/isobuild/bundler.js | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 60cb232e8e..47ea373311 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -176,6 +176,9 @@ import { PackageRegistry } from "../../packages/meteor/define-package.js"; const SOURCE_URL_PREFIX = "meteor://\u{1f4bb}app"; +const MINIFY_PLAIN_FUNCTION = Buffer.from('function(', 'utf8'); +const MINIFY_RENAMED_FUNCTION = Buffer.from('function __minifyJs(', 'utf8'); + // files to ignore when bundling. node has no globs, so use regexps exports.ignoreFiles = [ /~$/, /^\.#/, @@ -1333,13 +1336,10 @@ class Target { // expression, which some minifiers (e.g. UglifyJS) either fail to // parse or mistakenly eliminate as dead code. To avoid these // problems, we temporarily name the function __minifyJs. - file._contents = Buffer.from( - file.contents() - .toString("utf8") - .replace(/^\s*function\s*\(/, - "function __minifyJs("), - "utf8" - ); + file._contents = Buffer.concat([ + MINIFY_RENAMED_FUNCTION, + file.contents().slice(MINIFY_PLAIN_FUNCTION.length) + ]); dynamicFiles.push(jsf); @@ -1371,15 +1371,24 @@ class Target { function handle(source, dynamic) { source._minifiedFiles.forEach(file => { // Remove the function name __minifyJs that was added above. - file.data = file.data - .toString("utf8") - .replace(/^\s*function\s+__minifyJs\s*\(/, - "function("); + if (typeof file.data === 'string') { + file.data = Buffer.from( + file.data + .replace(/^\s*function\s+__minifyJs\s*\(/, + "function("), + "utf8" + ); + } else if (dynamic) { + file.data = Buffer.concat([ + MINIFY_PLAIN_FUNCTION, + file.data.slice(MINIFY_RENAMED_FUNCTION.length) + ]); + } const newFile = new File({ info: 'minified js', arch, - data: Buffer.from(file.data, 'utf8'), + data: file.data, hash: inputHashesByJsFile.get(source), }); From 29b6ca64835e0c014b94110276eb49011da38faf Mon Sep 17 00:00:00 2001 From: zodern Date: Tue, 5 Feb 2019 14:13:38 -0600 Subject: [PATCH 3/6] Avoid unnecessary work if the file was already written. --- tools/isobuild/builder.js | 44 ++++++++++++++++++++++++++++++--------- tools/isobuild/bundler.js | 4 ++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index 707dbb3262..e75ddb7493 100644 --- a/tools/isobuild/builder.js +++ b/tools/isobuild/builder.js @@ -231,6 +231,39 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` return partsOut.join(files.pathSep); } + // Checks if a file with the same path and hash was written by + // the previous builder. If it was, it adds it to the cache. + // It also makes sure the parent directories exist and are part of the cache. + // + // Returns true if the file was already written + usePreviousWrite (relPath, hash, sanitize) { + relPath = this._normalizeFilePath(relPath, sanitize); + this._ensureDirectory(files.pathDirname(relPath)); + + const previouslyWritten = this.previousWrittenHashes[relPath] === hash; + + if (previouslyWritten) { + this.writtenHashes[relPath] = hash; + this.usedAsFile[relPath] = true; + } + + return previouslyWritten; + } + + _normalizeFilePath (relPath, sanitize) { + // Ensure no trailing slash + if (relPath.slice(-1) === files.pathSep) { + relPath = relPath.slice(0, -1); + } + + // In sanitize mode, ensure path does not contain segments like + // '..', does not contain forbidden characters, and is unique. + if (sanitize) { + relPath = this._sanitize(relPath); + } + + return relPath; + } // Write either a buffer or the contents of a file to `relPath` (a // path to a file relative to the bundle root), creating it (and any @@ -254,16 +287,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` // // If `file` is used then it will be added to the builder's WatchSet. write(relPath, {data, file, hash, sanitize, executable, symlink}) { - // Ensure no trailing slash - if (relPath.slice(-1) === files.pathSep) { - relPath = relPath.slice(0, -1); - } - - // In sanitize mode, ensure path does not contain segments like - // '..', does not contain forbidden characters, and is unique. - if (sanitize) { - relPath = this._sanitize(relPath); - } + relPath = this._normalizeFilePath(relPath, sanitize); let getData = null; if (data) { diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 47ea373311..dac9a40311 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -2760,6 +2760,10 @@ var writeFile = Profile("bundler writeFile", function (file, builder, options) { let data = file.contents(); const hash = file.hash(); + if (builder.usePreviousWrite(file.targetPath, hash)) { + return; + } + if (options && options.sourceMapUrl) { data = addSourceMappingURL(data, options.sourceMapUrl); } else { From c34a61a3f923c2d446771c4b31ba88f28a238900 Mon Sep 17 00:00:00 2001 From: zodern Date: Tue, 12 Feb 2019 16:08:04 -0600 Subject: [PATCH 4/6] Simplify deciding when to not calculate SRI. --- tools/isobuild/bundler.js | 4 ++-- tools/isobuild/compiler.js | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index dac9a40311..c435af37ab 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -549,7 +549,7 @@ class File { // disk). this.sourcePath = options.sourcePath; - // Allows not calculating sri when the file doesn't support it + // Allows not calculating sri when it isn't needed this._skipSri = options.skipSri // info is just for help with debugging the tool; it isn't written to disk or @@ -1169,7 +1169,7 @@ class Target { cacheable: false, hash: resource.hash, sourcePath: resource.sourcePath, - skipSri: !!resource.sourcePath && resource.hash + skipSri: !!resource.hash }; const file = new File(fileOptions); diff --git a/tools/isobuild/compiler.js b/tools/isobuild/compiler.js index 91118ba9ab..4f8c5a2ca3 100644 --- a/tools/isobuild/compiler.js +++ b/tools/isobuild/compiler.js @@ -457,7 +457,7 @@ var compileUnibuild = Profile(function (options) { // This function needs to be factored out to support legacy handlers later on // in the compilation process - function addAsset(contents, relPath, absPath, hash) { + function addAsset(contents, relPath, hash) { // XXX hack to strip out private and public directory names from app asset // paths if (! inputSourceArch.pkg.name) { @@ -470,8 +470,7 @@ var compileUnibuild = Profile(function (options) { path: relPath, servePath: colonConverter.convert( files.pathJoin(inputSourceArch.pkg.serveRoot, relPath)), - hash: hash, - sourcePath: absPath + hash: hash }); } @@ -483,7 +482,7 @@ var compileUnibuild = Profile(function (options) { const hash = optimisticHashOrNull(absPath) const contents = optimisticReadFile(absPath) - addAsset(contents, relPath, absPath, hash); + addAsset(contents, relPath, hash); }); // Add and compile all source files From aadbbfbd2ba0f030f5e424c82cfbe7cea0d1152c Mon Sep 17 00:00:00 2001 From: zodern Date: Wed, 13 Feb 2019 15:25:26 -0600 Subject: [PATCH 5/6] Clean up code. --- tools/isobuild/builder.js | 9 +++++---- tools/isobuild/bundler.js | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index e75ddb7493..9f1397c593 100644 --- a/tools/isobuild/builder.js +++ b/tools/isobuild/builder.js @@ -231,18 +231,19 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` return partsOut.join(files.pathSep); } + // Checks if a file with the same path and hash was written by - // the previous builder. If it was, it adds it to the cache. - // It also makes sure the parent directories exist and are part of the cache. - // + // the previous builder. If it was, it adds it to the cache and makes + // sure the parent directories exist and are part of the cache. + // // Returns true if the file was already written usePreviousWrite (relPath, hash, sanitize) { relPath = this._normalizeFilePath(relPath, sanitize); - this._ensureDirectory(files.pathDirname(relPath)); const previouslyWritten = this.previousWrittenHashes[relPath] === hash; if (previouslyWritten) { + this._ensureDirectory(files.pathDirname(relPath)); this.writtenHashes[relPath] = hash; this.usedAsFile[relPath] = true; } diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index c435af37ab..1adbfe88ae 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1168,7 +1168,6 @@ class Target { data: resource.data, cacheable: false, hash: resource.hash, - sourcePath: resource.sourcePath, skipSri: !!resource.hash }; From 01f515e6b2fe486393690d6c422c5702128a3630 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Feb 2019 14:01:47 -0500 Subject: [PATCH 6/6] Inconsequential style tweaks. --- tools/isobuild/builder.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index 9f1397c593..ea71eb8fc1 100644 --- a/tools/isobuild/builder.js +++ b/tools/isobuild/builder.js @@ -237,21 +237,20 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` // sure the parent directories exist and are part of the cache. // // Returns true if the file was already written - usePreviousWrite (relPath, hash, sanitize) { + usePreviousWrite(relPath, hash, sanitize) { relPath = this._normalizeFilePath(relPath, sanitize); - const previouslyWritten = this.previousWrittenHashes[relPath] === hash; - - if (previouslyWritten) { + if (this.previousWrittenHashes[relPath] === hash) { this._ensureDirectory(files.pathDirname(relPath)); this.writtenHashes[relPath] = hash; this.usedAsFile[relPath] = true; + return true; } - return previouslyWritten; + return false; } - _normalizeFilePath (relPath, sanitize) { + _normalizeFilePath(relPath, sanitize) { // Ensure no trailing slash if (relPath.slice(-1) === files.pathSep) { relPath = relPath.slice(0, -1); @@ -316,7 +315,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` // Write is called multiple times for assets when they have multiple urls for the same file if (this.previousWrittenHashes[relPath] !== hash && this.writtenHashes[relPath] !== hash) { - + // Builder is used to create build products, which should be read-only; // users shouldn't be manually editing automatically generated files and // expecting the results to "stick".