From e85c69680dad10d7bb1be3a28ada62d2b114212e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 2 Oct 2017 13:41:39 -0400 Subject: [PATCH] Combine all isopackets to share transitive dependencies. (#9168) By my calculations, the sum of the sizes of the individual isopackets was 152MB, and the size of the combined isopacket is now just 36MB. That remarkable difference goes to show how much duplication of transitive dependencies was happening before this change. That's a savings of 116MB for the (uncompressed) size of the meteor-tool package. In Meteor 1.5.x, the meteor-tool package is about 544MB, but in Meteor 1.6 it's considerably smaller: 373MB. In other words, this change should reduce those sizes to 428MB (-21%) and 257MB (-31%), respectively. --- tools/cli/commands-packages-query.js | 8 +-- tools/cordova/builder.js | 12 ++--- tools/cordova/run-targets.js | 5 +- tools/isobuild/bundler.js | 5 +- tools/isobuild/isopack.js | 9 ++-- tools/meteor-services/auth.js | 14 +++--- tools/meteor-services/service-connection.js | 11 ++--- tools/project-context.js | 23 ++++----- tools/runners/run-log.js | 5 +- tools/runners/run-mongo.js | 16 +++--- tools/tool-env/isopackets.js | 55 ++++++++++++++------- tools/tool-testing/selftest.js | 5 +- tools/tool-testing/test-utils.js | 14 ++---- 13 files changed, 96 insertions(+), 86 deletions(-) diff --git a/tools/cli/commands-packages-query.js b/tools/cli/commands-packages-query.js index 370d56f663..d0f996fa97 100644 --- a/tools/cli/commands-packages-query.js +++ b/tools/cli/commands-packages-query.js @@ -6,7 +6,7 @@ var buildmessage = require('../utils/buildmessage.js'); var catalog = require('../packaging/catalog/catalog.js'); var Console = require('../console/console.js').Console; var files = require('../fs/files.js'); -var isopackets = require('../tool-env/isopackets.js'); +import { loadIsopackage } from '../tool-env/isopackets.js'; var main = require('./main.js'); var packageVersionParser = require('../packaging/package-version-parser.js'); var projectContextModule = require('../project-context.js'); @@ -111,10 +111,10 @@ var formatHiddenVersions = function (hiddenVersions, oldestShownVersion) { }; // Converts an object to an EJSON string with the right spacing. -var formatEJSON = function (data) { - var EJSON = isopackets.load('ejson').ejson.EJSON; +function formatEJSON(data) { + const { EJSON } = loadIsopackage('ejson'); return EJSON.stringify(data, { indent: true }) + "\n"; -}; +} // Takes in a string and pads it with whitespace to the length of the longest // possible date string. diff --git a/tools/cordova/builder.js b/tools/cordova/builder.js index 28049c231b..ba643a02ad 100644 --- a/tools/cordova/builder.js +++ b/tools/cordova/builder.js @@ -6,7 +6,7 @@ import files from '../fs/files.js'; import bundler from '../isobuild/bundler.js'; import archinfo from '../utils/archinfo.js'; import release from '../packaging/release.js'; -import { load as loadIsopacket } from '../tool-env/isopackets.js'; +import { loadIsopackage } from '../tool-env/isopackets.js'; import utils from '../utils/utils.js'; import { CORDOVA_ARCH } from './index.js'; @@ -231,7 +231,7 @@ export class CordovaBuilder { } writeConfigXmlAndCopyResources(shouldCopyResources = true) { - const { XmlBuilder } = loadIsopacket('cordova-support')['xmlbuilder']; + const { XmlBuilder } = loadIsopackage('xmlbuilder'); let config = XmlBuilder.create('widget'); @@ -409,8 +409,8 @@ export class CordovaBuilder { let configDummy = {}; configDummy.PUBLIC_SETTINGS = publicSettings || {}; - const { WebAppHashing } = - loadIsopacket('cordova-support')['webapp-hashing']; + const { WebAppHashing } = loadIsopackage('webapp-hashing'); + program.version = WebAppHashing.calculateClientHash(program.manifest, null, configDummy); } @@ -442,8 +442,8 @@ export class CordovaBuilder { runtimeConfig.PUBLIC_SETTINGS = publicSettings; } - const { Boilerplate } = - loadIsopacket('cordova-support')['boilerplate-generator']; + const { Boilerplate } = loadIsopackage('boilerplate-generator'); + const boilerplate = new Boilerplate(CORDOVA_ARCH, manifest, { urlMapper: _.identity, pathMapper: (path) => files.convertToOSPath( diff --git a/tools/cordova/run-targets.js b/tools/cordova/run-targets.js index 80bb118f0e..76b60d0e1b 100644 --- a/tools/cordova/run-targets.js +++ b/tools/cordova/run-targets.js @@ -2,7 +2,7 @@ import _ from 'underscore'; import chalk from 'chalk'; import child_process from 'child_process'; -import { load as loadIsopacket } from '../tool-env/isopackets.js'; +import { loadIsopackage } from '../tool-env/isopackets.js'; import runLog from '../runners/run-log.js'; import { Console } from '../console/console.js'; import files from '../fs/files.js'; @@ -144,8 +144,7 @@ export class AndroidRunTarget extends CordovaRunTarget { `CordovaLog:${logLevel}`, `chromium:${logLevel}`, `SystemWebViewClient:${logLevel}`, '*:F']; - const { Log } = - loadIsopacket('cordova-support')['logging']; + const { Log } = loadIsopackage('logging'); const logStream = transform(line => { const logEntry = logFromAndroidLogcatLine(Log, line); diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 2dea5b3db0..5a4369c474 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -169,7 +169,7 @@ var colonConverter = require('../utils/colon-converter.js'); var Profile = require('../tool-env/profile.js').Profile; var packageVersionParser = require('../packaging/package-version-parser.js'); var release = require('../packaging/release.js'); -import { load as loadIsopacket } from '../tool-env/isopackets.js'; +import { loadIsopackage } from '../tool-env/isopackets.js'; import { CORDOVA_PLATFORM_VERSIONS } from '../cordova'; import { gzipSync } from "zlib"; @@ -1673,8 +1673,7 @@ class ClientTarget extends Target { }; if (this.arch === 'web.cordova') { - const { WebAppHashing } = - loadIsopacket('cordova-support')['webapp-hashing']; + const { WebAppHashing } = loadIsopackage('webapp-hashing'); const cordovaCompatibilityVersions = _.object(_.map(CORDOVA_PLATFORM_VERSIONS, (version, platform) => { diff --git a/tools/isobuild/isopack.js b/tools/isobuild/isopack.js index cbba698183..0b4f7db297 100644 --- a/tools/isobuild/isopack.js +++ b/tools/isobuild/isopack.js @@ -8,7 +8,10 @@ import Builder from './builder.js'; var bundler = require('./bundler.js'); var watch = require('../fs/watch.js'); var files = require('../fs/files.js'); -var isopackets = require('../tool-env/isopackets.js'); +import { + ISOPACKETS, + makeIsopacketBuildContext, +} from '../tool-env/isopackets.js'; var colonConverter = require('../utils/colon-converter.js'); var utils = require('../utils/utils.js'); var buildPluginModule = require('./build-plugin.js'); @@ -1807,14 +1810,14 @@ _.extend(Isopack.prototype, { // Build all of the isopackets now, so that no build step is required when // you're actually running meteor from a release in order to load packages. - var isopacketBuildContext = isopackets.makeIsopacketBuildContext(); + var isopacketBuildContext = makeIsopacketBuildContext(); var messages = buildmessage.capture(function () { // We rebuild them in the order listed in ISOPACKETS. This is not strictly // necessary here, since any isopackets loaded as part of the build // process are going to be the current tool's isopackets, not the // isopackets that we're writing out. - _.each(isopackets.ISOPACKETS, function (packages, isopacketName) { + _.each(ISOPACKETS, function (packages, isopacketName) { requestGarbageCollection(); buildmessage.enterJob({ diff --git a/tools/meteor-services/auth.js b/tools/meteor-services/auth.js index 3660f42934..2d68f5fb3e 100644 --- a/tools/meteor-services/auth.js +++ b/tools/meteor-services/auth.js @@ -6,20 +6,20 @@ var httpHelpers = require('../utils/http-helpers.js'); var fiberHelpers = require('../utils/fiber-helpers.js'); var querystring = require('querystring'); var url = require('url'); -var isopackets = require('../tool-env/isopackets.js'); var Console = require('../console/console.js').Console; var auth = exports; -var getLoadedPackages = function () { - return isopackets.load('ddp'); -}; +function loadDDP() { + return require("../tool-env/isopackets.js") + .loadIsopackage("ddp-client") + .DDP; +} // Opens and returns a DDP connection to the accounts server. Remember // to close it when you're done with it! var openAccountsConnection = function () { - var DDP = getLoadedPackages()['ddp-client'].DDP; - return DDP.connect(config.getAuthDDPUrl(), { + return loadDDP().connect(config.getAuthDDPUrl(), { headers: { 'User-Agent': httpHelpers.getUserAgent() } }); }; @@ -47,7 +47,7 @@ var withAccountsConnection = function (f) { // XXX if we reconnect we won't reauthenticate. Fix that before using // this for long-lived connections. var loggedInAccountsConnection = function (token) { - var connection = getLoadedPackages()['ddp-client'].DDP.connect( + var connection = loadDDP().connect( config.getAuthDDPUrl() ); diff --git a/tools/meteor-services/service-connection.js b/tools/meteor-services/service-connection.js index 53e55a4bef..7cbc80af12 100644 --- a/tools/meteor-services/service-connection.js +++ b/tools/meteor-services/service-connection.js @@ -1,5 +1,5 @@ var _ = require("underscore"); -var isopackets = require('../tool-env/isopackets.js'); +import { loadIsopackage } from '../tool-env/isopackets.js'; var files = require('../fs/files.js'); var fiberHelpers = require("../utils/fiber-helpers.js"); @@ -24,9 +24,8 @@ var fiberHelpers = require("../utils/fiber-helpers.js"); // ...and anything else you'd normally pass as options to DDP.connect // var ServiceConnection = function (endpointUrl, options) { - var self = this; - - var Package = isopackets.load('ddp'); + const self = this; + const ddpClient = loadIsopackage('ddp-client'); // ServiceConnection never should retry connections: just one TCP connection // is enough, and any errors on it should be detected promptly. @@ -60,7 +59,7 @@ var ServiceConnection = function (endpointUrl, options) { } } - self.connection = Package['ddp-client'].DDP.connect(endpointUrl, options); + self.connection = ddpClient.DDP.connect(endpointUrl, options); // Wait until we have some sort of initial connection or error (including the // 10-second timeout built into our DDP client). @@ -87,7 +86,7 @@ var ServiceConnection = function (endpointUrl, options) { var promise = self.currentPromise; self.currentPromise = null; promise.reject( - error || new Package['ddp-client'].DDP.ConnectionError( + error || new ddpClient.DDP.ConnectionError( "DDP disconnected while connection in progress") ); } else if (error) { diff --git a/tools/project-context.js b/tools/project-context.js index e2071e80d1..f9cbfde1eb 100644 --- a/tools/project-context.js +++ b/tools/project-context.js @@ -8,7 +8,7 @@ var catalogLocal = require('./packaging/catalog/catalog-local.js'); var Console = require('./console/console.js').Console; var files = require('./fs/files.js'); var isopackCacheModule = require('./isobuild/isopack-cache.js'); -var isopackets = require('./tool-env/isopackets.js'); +import { loadIsopackage } from './tool-env/isopackets.js'; var packageMapModule = require('./packaging/package-map.js'); var release = require('./packaging/release.js'); var tropohouse = require('./packaging/tropohouse.js'); @@ -789,20 +789,15 @@ _.extend(ProjectContext.prototype, { }, _buildResolver: function () { - var self = this; + const { ConstraintSolver } = loadIsopackage('constraint-solver'); - var constraintSolverPackage = - isopackets.load('constraint-solver')['constraint-solver']; - var resolver = - new constraintSolverPackage.ConstraintSolver.PackagesResolver( - self.projectCatalog, { - nudge: function () { - Console.nudge(true); - }, - Profile: Profile, - resultCache: self._resolverResultCache - }); - return resolver; + return new ConstraintSolver.PackagesResolver(this.projectCatalog, { + nudge() { + Console.nudge(true); + }, + Profile: Profile, + resultCache: this._resolverResultCache + }); }, _downloadMissingPackages: Profile('_downloadMissingPackages', function () { diff --git a/tools/runners/run-log.js b/tools/runners/run-log.js index d3b54d18f5..a40da09540 100644 --- a/tools/runners/run-log.js +++ b/tools/runners/run-log.js @@ -1,5 +1,4 @@ var _ = require('underscore'); -var isopackets = require('../tool-env/isopackets.js'); var Console = require('../console/console.js').Console; var fiberHelpers = require('../utils/fiber-helpers.js'); @@ -20,7 +19,9 @@ var fiberHelpers = require('../utils/fiber-helpers.js'); let _Log; function getLoggingPackage() { if (! _Log) { - _Log = isopackets.load('logging').logging.Log; + _Log = require("../tool-env/isopackets.js") + .loadIsopackage('logging') + .Log; } // Since no other process will be listening to stdout and parsing it, diff --git a/tools/runners/run-mongo.js b/tools/runners/run-mongo.js index 434612be32..b72e73da23 100644 --- a/tools/runners/run-mongo.js +++ b/tools/runners/run-mongo.js @@ -6,7 +6,7 @@ var runLog = require('./run-log.js'); var child_process = require('child_process'); var _ = require('underscore'); -var isopackets = require('../tool-env/isopackets.js'); +import { loadIsopackage } from '../tool-env/isopackets.js'; var Console = require('../console/console.js').Console; // Given a Mongo URL, open an interative Mongo shell on this terminal @@ -585,17 +585,19 @@ var launchMongo = function (options) { var initiateReplSetAndWaitForReady = function () { try { // Load mongo so we'll be able to talk to it. - var mongoNpmModule = - isopackets.load('mongo')['npm-mongo'].NpmModuleMongodb; + const { Db, Server } = loadIsopackage('npm-mongo').NpmModuleMongodb; // Connect to the intended primary and start a replset. - var db = new mongoNpmModule.Db( + var db = new Db( 'meteor', - new mongoNpmModule.Server('127.0.0.1', options.port, { + new Server('127.0.0.1', options.port, { poolSize: 1, - socketOptions: {connectTimeoutMS: 60000}, + socketOptions: { + connectTimeoutMS: 60000 + } }), - {safe: true}); + { safe: true } + ); yieldingMethod(db, 'open'); if (stopped) { diff --git a/tools/tool-env/isopackets.js b/tools/tool-env/isopackets.js index 0631237436..fe5035f9cf 100644 --- a/tools/tool-env/isopackets.js +++ b/tools/tool-env/isopackets.js @@ -42,7 +42,7 @@ var Profile = require('./profile.js').Profile; // process if any of their sources have changed. // // Example usage: -// var DDP = require('./isopackets.js').load('ddp')['ddp-client'].DDP; +// var DDP = require('./isopackets.js').loadIsopackage('ddp-client').DDP; // var reverse = DDP.connect('reverse.meteor.com'); // Console.info(reverse.call('reverse', 'hello world')); @@ -50,13 +50,26 @@ var Profile = require('./profile.js').Profile; // All of the defined isopackets. Whenever they are being built, they will be // built in the order listed here. export const ISOPACKETS = { - 'ddp': ['ddp-client'], - 'mongo': ['npm-mongo'], - 'ejson': ['ejson'], - 'constraint-solver': ['constraint-solver'], - 'cordova-support': ['boilerplate-generator', 'logging', 'webapp-hashing', - 'xmlbuilder'], - 'logging': ['logging'], + // These packages used to be divided up into distinct isopackets, but + // that resulted in extremely wasteful duplication of transitive + // dependencies, so now we have only one isopacket that combines all the + // dependencies of every former isopacket. + combined: [ + // ddp + 'ddp-client', + // mongo + 'npm-mongo', + // ejson + 'ejson', + // constraint-solver + 'constraint-solver', + // cordova-support + 'boilerplate-generator', + 'webapp-hashing', + 'xmlbuilder', + // cordova-support, logging + 'logging', + ] }; // Caches isopackets in memory (each isopacket only needs to be loaded @@ -75,17 +88,19 @@ export const ISOPACKETS = { // the tool itself. var loadedIsopackets = {}; -// The main entry point: loads and returns an isopacket from cache or from -// disk. Does not do a build step: ensureIsopacketsLoadable must be called -// first! -export function load(isopacketName) { +// The main entry point: loads the specified isopacket ("combined" by +// default) from cache or from disk, and returns the requested package +// dependency, complaining if the package does not exist. Note that +// ensureIsopacketsLoadable must be called first, as this function does +// not trigger any building. +export function loadIsopackage(packageName, isopacketName = "combined") { // Small but necessary hack: because archinfo.host() calls execFileSync, // it yields the first time we call it, which is a problem for the // fiberHelpers.noYieldsAllowed block below. Calling it here ensures the // result is cached, so no yielding occurs later. assert.strictEqual(archinfo.host().split(".", 1)[0], "os"); - return fiberHelpers.noYieldsAllowed(function () { + const isopacket = fiberHelpers.noYieldsAllowed(function () { if (_.has(loadedIsopackets, isopacketName)) { if (loadedIsopackets[isopacketName]) { return loadedIsopackets[isopacketName]; @@ -93,9 +108,8 @@ export function load(isopacketName) { // This is the case where the isopacket is up to date on disk but not // loaded. - var isopacket = loadIsopacketFromDisk(isopacketName); - loadedIsopackets[isopacketName] = isopacket; - return isopacket; + return loadedIsopackets[isopacketName] = + loadIsopacketFromDisk(isopacketName); } if (_.has(ISOPACKETS, isopacketName)) { @@ -105,6 +119,12 @@ export function load(isopacketName) { throw Error("Unknown isopacket: " + isopacketName); }); + + if (! _.has(isopacket, packageName)) { + throw new Error("Unknown isopacket dependency: " + packageName); + } + + return isopacket[packageName]; } var isopacketPath = function (isopacketName) { @@ -309,6 +329,3 @@ var loadIsopacketFromDisk = function (isopacketName) { return ret; }; - -// Support `import isopackets from "../path/to/isopackets.js"`. -export default exports; diff --git a/tools/tool-testing/selftest.js b/tools/tool-testing/selftest.js index 4935d531d0..4a60a52411 100644 --- a/tools/tool-testing/selftest.js +++ b/tools/tool-testing/selftest.js @@ -25,7 +25,7 @@ import Builder from '../isobuild/builder.js'; import { DEFAULT_TRACK } from '../packaging/catalog/catalog.js'; import { RemoteCatalog } from '../packaging/catalog/catalog-remote.js'; import { IsopackCache } from '../isobuild/isopack-cache.js'; -import { load as isoPacketsLoad } from '../tool-env/isopackets.js'; +import { loadIsopackage } from '../tool-env/isopackets.js'; import { Tropohouse } from '../packaging/tropohouse.js'; import { PackageMap } from '../packaging/package-map.js'; import { current as releaseCurrent } from '../packaging/release.js'; @@ -85,8 +85,7 @@ export const fail = markStack(function (reason) { // with 'actual' being the value that the test got and 'expected' // being the expected value export const expectEqual = markStack(function (actual, expected) { - const Package = isoPacketsLoad('ejson'); - if (! Package.ejson.EJSON.equals(actual, expected)) { + if (! loadIsopackage('ejson').EJSON.equals(actual, expected)) { throw new TestFailure("not-equal", { expected: expected, actual: actual diff --git a/tools/tool-testing/test-utils.js b/tools/tool-testing/test-utils.js index 7f5f18744e..ba493ac5d1 100644 --- a/tools/tool-testing/test-utils.js +++ b/tools/tool-testing/test-utils.js @@ -1,4 +1,3 @@ -var isopackets = require('../tool-env/isopackets.js'); var config = require('../meteor-services/config.js'); var utils = require('../utils/utils.js'); var auth = require('../meteor-services/auth.js'); @@ -6,6 +5,8 @@ var selftest = require('./selftest.js'); var httpHelpers = require('../utils/http-helpers.js'); var _ = require('underscore'); +import { loadIsopackage } from '../tool-env/isopackets.js' + var randomString = function (charsCount) { var chars = 'abcdefghijklmnopqrstuvwxyz'; var str = ''; @@ -52,14 +53,9 @@ var registrationUrlRegexp = /https:\/\/www\.meteor\.com\/setPassword\?([a-zA-Z0-9\+\/]+)/; exports.registrationUrlRegexp = registrationUrlRegexp; -var getLoadedPackages = function () { - return isopackets.load('ddp'); -}; - -var ddpConnect = function (url) { - var DDP = getLoadedPackages()['ddp-client'].DDP; - return DDP.connect(url); -}; +function ddpConnect(url) { + return loadIsopackage('ddp-client').DDP.connect(url); +} exports.ddpConnect = ddpConnect;