From 6229252bfb6711115c77acfc4603af33cd4654bd Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 18 Mar 2013 13:34:57 -0700 Subject: [PATCH] simplify parallel package download code and make files.getUrl synchronous --- engine/fiber-helpers.js | 13 ++++++++++++ engine/files.js | 15 ++++++++++---- engine/meteor_npm.js | 2 +- engine/updater.js | 2 +- engine/warehouse.js | 46 ++++++++++++++++++----------------------- 5 files changed, 46 insertions(+), 32 deletions(-) diff --git a/engine/fiber-helpers.js b/engine/fiber-helpers.js index eb1e264b96..25db14b8e4 100644 --- a/engine/fiber-helpers.js +++ b/engine/fiber-helpers.js @@ -6,7 +6,9 @@ // meteor apps. Error.stackTraceLimit = Infinity; // http://code.google.com/p/v8/wiki/JavaScriptStackTraceApi +var _ = require("underscore"); var Fiber = require("fibers"); +var Future = require("fibers/future"); // runs a function within a fiber. we wrap the entry point into // meteor.js into a fiber but if you use callbacks that call @@ -24,3 +26,14 @@ exports.inFiber = function(func) { }; }; +exports.parallelMap = function (collection, callback, context) { + var futures = _.map(collection, function () { + var args = _.toArray(arguments); + return function () { + return callback.apply(context, args); + }.future()(); + }); + Future.wait(futures); + // Throw if any threw. + return _.map(futures, function (f) { return f.get(); }); +}; diff --git a/engine/files.js b/engine/files.js index 678e3f14f5..a8f2833ed0 100644 --- a/engine/files.js +++ b/engine/files.js @@ -413,12 +413,19 @@ var files = module.exports = { tar.Pack()).pipe(zlib.createGzip()); }, - // A thin wrapper around request(...) that makes the response "body" - // the main callback argument. This facilitates using `Future.wrap`. + // A synchronous wrapper around request(...) that returns the response "body" + // or throws. getUrl: function (urlOrOptions, callback) { - return request(urlOrOptions, function (error, response, body) { - callback(error, body, response); + var future = new Future; + // can't just use Future.wrap, because we want to return "body", not + // "response". + request(urlOrOptions, function (error, response, body) { + if (error) + future.throw(error); + else + future.return(body); }); + return future.wait(); }, _randomToken: function() { diff --git a/engine/meteor_npm.js b/engine/meteor_npm.js index 4806c64832..c766bc79b3 100644 --- a/engine/meteor_npm.js +++ b/engine/meteor_npm.js @@ -270,7 +270,7 @@ var meteorNpm = module.exports = { // dependencies. `npm install` times out after more than a minute. _ensureConnected: function () { try { - Future.wrap(files.getUrl)("http://registry.npmjs.org").wait(); + files.getUrl("http://registry.npmjs.org"); } catch (e) { throw new Error( "Can't install npm dependencies. Check your internet connection and try again."); diff --git a/engine/updater.js b/engine/updater.js index ac41da4c64..369ba47ff6 100644 --- a/engine/updater.js +++ b/engine/updater.js @@ -22,7 +22,7 @@ var manifestUrl = testingUpdater * null on error) */ exports.getManifest = function () { - return Future.wrap(files.getUrl)({url: manifestUrl, json: true}).wait(); + return files.getUrl({url: manifestUrl, json: true}); }; exports.git_sha = function () { diff --git a/engine/warehouse.js b/engine/warehouse.js index 8500e208a3..4c9e3cf407 100644 --- a/engine/warehouse.js +++ b/engine/warehouse.js @@ -27,6 +27,7 @@ var _ = require("underscore"); var files = require('./files.js'); var updater = require('./updater.js'); +var fiberHelpers = require('./fiber-helpers.js'); var PACKAGES_URLBASE = 'https://warehouse.meteor.com'; @@ -178,8 +179,8 @@ var warehouse = module.exports = { // writing packages var releaseManifest; try { - releaseManifest = JSON.parse(Future.wrap(files.getUrl)( - PACKAGES_URLBASE + "/releases/" + releaseVersion + ".release.json").wait()); + releaseManifest = JSON.parse(files.getUrl( + PACKAGES_URLBASE + "/releases/" + releaseVersion + ".release.json")); } catch (e) { if (background) throw e; // just throw, it's being ignored @@ -192,8 +193,8 @@ var warehouse = module.exports = { // try getting the releases's changelog. notable only blessed // releases have one, so if we can't find it just proceed try { - var changelog = Future.wrap(files.getUrl)( - PACKAGES_URLBASE + "/releases/" + releaseVersion + ".changelog.json").wait(); + var changelog = files.getUrl( + PACKAGES_URLBASE + "/releases/" + releaseVersion + ".changelog.json"); // If a file is not on S3 we get served an 'access denied' XML // file. This will throw (intentionally) in that case. Real @@ -221,10 +222,10 @@ var warehouse = module.exports = { + engineTarballFilename; if (!background) console.log("Fetching Meteor Engine " + engineVersion + "..."); - var engineTarball = Future.wrap(files.getUrl)({ + var engineTarball = files.getUrl({ url: PACKAGES_URLBASE + engineTarballPath, encoding: null - }).wait(); + }); files.extractTarGz(engineTarball, warehouse.getEngineDir(engineVersion)); } catch (e) { @@ -291,29 +292,22 @@ var warehouse = module.exports = { // @param packagesToPopulate {Object} eg {"less": "0.5.0"} _populateWarehouseWithPackages: function(packagesToPopulate, background) { - var futures = []; - _.each(packagesToPopulate, function (version, name) { - var packageDir = path.join(warehouse.getWarehouseDir(), 'packages', name, version); - var packageUrl = PACKAGES_URLBASE + "/packages/" + name + "/" + - name + '-' + version + ".tar.gz"; + var results = fiberHelpers.parallelMap( + packagesToPopulate, function (version, name) { + var packageDir = path.join(warehouse.getWarehouseDir(), 'packages', + name, version); + var packageUrl = PACKAGES_URLBASE + "/packages/" + name + "/" + + name + '-' + version + ".tar.gz"; - if (!background) - console.log("Fetching " + packageUrl + "..."); - futures.push(Future.wrap(function (cb) { - files.getUrl({url: packageUrl, encoding: null}, function (error, result) { - if (! error && result) - result = { buffer: result, packageDir: packageDir, name: name }; - cb(error, result); - }); - })()); - }); + if (!background) + console.log("Fetching " + packageUrl + "..."); - Future.wait(futures); - - _.each(futures, function (f) { - var result = f.get(); - files.extractTarGz(result.buffer, result.packageDir); + var tarball = files.getUrl({url: packageUrl, encoding: null}); + files.extractTarGz(tarball, packageDir); + return {name: name, packageDir: packageDir}; + }); + _.each(results, function (result) { // fetch npm dependencies var packages = require(path.join(__dirname, "packages.js")); // load late to work around circular require var pkg = packages.loadFromDir(result.name, result.packageDir);