From 5deccfd1ef80ffd9a842bb760c688136ee601812 Mon Sep 17 00:00:00 2001 From: Chad Phillips Date: Sat, 26 Mar 2016 15:34:23 -0600 Subject: [PATCH] cleanup on 83c0f41e4c, startup performance regression this cleanup uses standard backbone architecture to avoid sorting prior to all collections being loaded. i've included a note for future developers about the issue, added some defensive coding for all other collections being loaded, to prevent similar issues in the future. --- lib/unhangout-db.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/unhangout-db.js b/lib/unhangout-db.js index 82bedbcb..69e2604b 100644 --- a/lib/unhangout-db.js +++ b/lib/unhangout-db.js @@ -9,7 +9,6 @@ var logger = require('./logging').getLogger(), var UnhangoutDb = function(options) { this.options = options; }; -var _origUsersComparator; _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { init: function(callback) { _.bindAll(this, "loadModels"); @@ -19,8 +18,6 @@ _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { this.options.UNHANGOUT_REDIS_HOST ); this.users = new models.ServerUserList(); - _origUsersComparator = this.users.comparator; - this.users.comparator = undefined; this.events = new models.ServerEventList(); this.permalinkSessions = new models.ServerSessionList(); @@ -97,19 +94,24 @@ _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { // representing each of the objects of that type in redis. It simply // needs to construct matching objects. To add a new type, just add a // matching entry in loaders and follow the format. + // NOTE: For any new types that are added, be sure to add {sort: false} + // to the options for the calls to set/add to objects in collections, + // and if a final sort is needed, call it on the collection after + // everything has been loaded -- otherwise potentially huge impacts on + // startup performance may occur! var that = this; var loaders = [ ["user/*", function(callback, attrs, key) { var newUser = new models.ServerUser(attrs); // no need to save since we're pulling from the // database to begin with. - that.users.add(newUser); + that.users.add(newUser, {sort: false}); callback(); }], ["event/?????", function(callback, attrs, key) { var newEvent = new models.ServerEvent(attrs); - that.events.add(newEvent); + that.events.add(newEvent, {sort: false}); callback(); }], @@ -134,7 +136,7 @@ _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { var newSession = new models.ServerSession(attrs, { collection: event.get("sessions") }); - event.get("sessions").add(newSession); + event.get("sessions").add(newSession, {sort: false}); // Reset state as needed. newSession.onRestart(); @@ -149,7 +151,7 @@ _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { // force these to be true. This fixes a transient condition where some // keys in the db didn't have this set and it defaults to false.dw newSession.set("isPermalinkSession", true); - that.permalinkSessions.add(newSession); + that.permalinkSessions.add(newSession, {sort: false}); // Reset state as needed. newSession.onRestart(); @@ -207,7 +209,6 @@ _.extend(UnhangoutDb.prototype, events.EventEmitter.prototype, { }; async.mapSeries(loaders, load, function(err, results) { logger.info("Done loading models."); - that.users.comparator = _origUsersComparator; that.users.sort(); callback(); });