From ce1954a9f7168331af66e093a7976e87e08e1d45 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 20 Dec 2017 12:17:58 +0000 Subject: [PATCH] Improved roster performance. Don't sort the roster group for each `chat_status` change. Instead batch every 500ms. --- spec/chatbox.js | 19 +++++----- spec/controlbox.js | 18 ++++++---- spec/profiling.js | 73 +++++++++++++++++++++++++++++--------- src/converse-headline.js | 5 ++- src/converse-rosterview.js | 25 ++++++++----- tests/runner.js | 2 +- 6 files changed, 100 insertions(+), 42 deletions(-) diff --git a/spec/chatbox.js b/spec/chatbox.js index ad742bcf1..3a2173f3c 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -73,10 +73,11 @@ spyOn(_converse.chatboxviews, 'trimChats'); expect($("#conversejs .chatbox").length).toBe(1); // Controlbox is open - var online_contacts = _converse.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact').find('a.open-chat'); + var online_contacts = _converse.rosterview.$el.find('.roster-group .current-xmpp-contact a.open-chat'); + expect(online_contacts.length).toBe(15); for (i=0; i 1; - }, 500); + return _converse.chatboxviews.keys().length > 1; + }, 500); }).then(function () { var key = _converse.chatboxviews.keys()[1]; trimmedview = trimmed_chatboxes.get(key); diff --git a/spec/controlbox.js b/spec/controlbox.js index ef4aeceeb..cc01f2b2e 100644 --- a/spec/controlbox.js +++ b/spec/controlbox.js @@ -418,9 +418,8 @@ }); } test_utils.waitUntil(function () { - return _converse.rosterview.$el.find('li:visible').length; - }, 500) - .then(function () { + return _converse.rosterview.$el.find('li:visible').length; + }, 500).then(function () { // Check that usernames appear alphabetically per group _.each(groups, function (name) { var $contacts = _converse.rosterview.$('.roster-group[data-group="'+name+'"] li'); @@ -954,8 +953,7 @@ _addContacts(_converse); test_utils.waitUntil(function () { return _converse.rosterview.$el.find('.roster-group').length; - }, 500) - .then(function () { + }, 500).then(function () { var i, jid; for (i=0; i<3; i++) { jid = mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost'; @@ -977,7 +975,15 @@ jid = mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost'; _converse.roster.get(jid).set('chat_status', 'unavailable'); } - + return test_utils.waitUntil(function () { + return _converse.rosterview.$el.find('li.online').length + }) + }).then(function () { + return test_utils.waitUntil(function () { + return _converse.rosterview.$el.find('li:first').text().trim() === 'Candice van der Knijff' + }, 900); + }).then(function () { + var i; var contacts = _converse.rosterview.$el.find('.current-xmpp-contact'); for (i=0; i<3; i++) { expect($(contacts[i]).hasClass('online')).toBeTruthy(); diff --git a/spec/profiling.js b/spec/profiling.js index 2c4d31114..5629fcb77 100644 --- a/spec/profiling.js +++ b/spec/profiling.js @@ -1,20 +1,21 @@ (function (root, factory) { - define(["mock", "converse-core", "test_utils"], factory); -} (this, function (mock, converse, test_utils) { + define(["jasmine", "mock", "converse-core", "test-utils", "utils"], factory); +} (this, function (jasmine, mock, converse, test_utils, u) { var _ = converse.env._; var $iq = converse.env.$iq; describe("Profiling", function() { - afterEach(function () { - converse.user.logout(); - test_utils.clearBrowserStorage(); - }); + xit("adds hundreds of contacts to the roster", + mock.initConverseWithPromises( + null, ['rosterGroupsFetched'], {}, + function (done, _converse) { - xit("adds hundreds of contacts to the roster", mock.initConverse(function(_converse) { _converse.roster_groups = false; - expect(this.roster.pluck('jid').length).toBe(0); + test_utils.openControlBox(); + + expect(_converse.roster.pluck('jid').length).toBe(0); var stanza = $iq({ - to: this.connection.jid, + to: _converse.connection.jid, type: 'result', id: 'roster_1' }).c('query', { @@ -29,16 +30,37 @@ }).c('group').t(group).up().up(); } }); - this.roster.onReceivedFromServer(stanza.tree()); - // expect(this.roster.pluck('jid').length).toBe(400); + _converse.roster.onReceivedFromServer(stanza.tree()); + + return test_utils.waitUntil(function () { + var $group = _converse.rosterview.$el.find('.roster-group') + return $group.length && u.isVisible($group[0]); + }).then(function () { + var count = 0; + _converse.roster.each(function (contact) { + if (count < 10) { + contact.set('chat_status', 'online'); + count += 1; + } + }); + return test_utils.waitUntil(function () { + return _converse.rosterview.$el.find('li.online').length + }) + }).then(done); })); - xit("adds hundreds of contacts to the roster, with roster groups", mock.initConverse(function(_converse) { + xit("adds hundreds of contacts to the roster, with roster groups", + mock.initConverseWithPromises( + null, ['rosterGroupsFetched'], {}, + function (done, _converse) { + // _converse.show_only_online_users = true; _converse.roster_groups = true; - expect(this.roster.pluck('jid').length).toBe(0); + test_utils.openControlBox(); + + expect(_converse.roster.pluck('jid').length).toBe(0); var stanza = $iq({ - to: this.connection.jid, + to: _converse.connection.jid, type: 'result', id: 'roster_1' }).c('query', { @@ -53,8 +75,27 @@ }).c('group').t(group).up().up(); } }); - this.roster.onReceivedFromServer(stanza.tree()); - //expect(this.roster.pluck('jid').length).toBe(400); + _converse.roster.onReceivedFromServer(stanza.tree()); + + return test_utils.waitUntil(function () { + var $group = _converse.rosterview.$el.find('.roster-group') + return $group.length && u.isVisible($group[0]); + }).then(function () { + _.each(['Friends', 'Colleagues', 'Family', 'Acquaintances'], function (group) { + var count = 0; + _converse.roster.each(function (contact) { + if (_.includes(contact.get('groups'), group)) { + if (count < 10) { + contact.set('chat_status', 'online'); + count += 1; + } + } + }); + }); + return test_utils.waitUntil(function () { + return _converse.rosterview.$el.find('li.online').length + }) + }).then(done); })); }); })); diff --git a/src/converse-headline.js b/src/converse-headline.js index efc0751e5..9809c85a4 100644 --- a/src/converse-headline.js +++ b/src/converse-headline.js @@ -109,7 +109,10 @@ this.content = this.$content[0]; utils.refreshWebkit(); return this; - } + }, + + // Override to avoid the method in converse-chatview.js + 'afterShown': _.noop }); function onHeadlineMessage (message) { diff --git a/src/converse-rosterview.js b/src/converse-rosterview.js index ff5a96700..a4737441d 100644 --- a/src/converse-rosterview.js +++ b/src/converse-rosterview.js @@ -678,13 +678,11 @@ }, initialize () { + this.sortEventually = _.debounce(this.sortAndPositionAll, 500); this.model.contacts.on("add", this.onContactAdded, this); this.model.contacts.on("change:subscription", this.onContactSubscriptionChange, this); this.model.contacts.on("change:requesting", this.onContactRequestChange, this); - this.model.contacts.on("change:chat_status", function (contact) { - this.model.contacts.sort(); - this.positionContact(contact).render(); - }, this); + this.model.contacts.on("change:chat_status", this.sortEventually, this); this.model.contacts.on("destroy", this.onRemove, this); this.model.contacts.on("remove", this.onRemove, this); _converse.roster.on('change:groups', this.onContactGroupChange, this); @@ -702,10 +700,15 @@ return this; }, - onContactAdded (contact) { - let contact_view = new _converse.RosterContactView({model: contact}); + createContactView (contact) { + const contact_view = new _converse.RosterContactView({model: contact}); this.add(contact.get('id'), contact_view); - contact_view = this.positionContact(contact).render(); + contact_view.render(); + return contact_view; + }, + + onContactAdded (contact) { + const contact_view = this.positionContact(contact); if (contact_view.mayBeShown()) { if (this.model.get('state') === _converse.CLOSED) { u.hideElement(contact_view.el); @@ -721,8 +724,7 @@ /* Place the contact's DOM element in the correct alphabetical * position amongst the other contacts in this group. */ - const view = this.get(contact.get('id')); - view.render(); + const view = this.get(contact.get('id')) || this.createContactView(contact); const list = this.contacts_el; const index = this.model.contacts.indexOf(contact); if (index === 0) { @@ -736,6 +738,11 @@ return view; }, + sortAndPositionAll () { + this.model.contacts.sort(); + this.model.contacts.each(this.positionContact.bind(this)); + }, + show () { u.showElement(this.el); _.each(this.getAll(), (contact_view) => { diff --git a/tests/runner.js b/tests/runner.js index 666137d3f..7d5849861 100644 --- a/tests/runner.js +++ b/tests/runner.js @@ -30,8 +30,8 @@ require.config(config); var specs = [ //"spec/transcripts", - // "spec/profiling", "jasmine", + "spec/profiling", "spec/utils", "spec/converse", "spec/bookmarks",