commit b02efd6b0cb5263c37bd81f6f16b4cdeba5b11b7 Author: Iain R. Learmonth irl@fsfe.org Date: Sun Nov 26 02:21:40 2017 +0000

Fixes long-lived performance bug; max result set size to 2000

* The Backbone.js collection for the search results was being updated for every relay "looked up", when really this should only have happened when all relays had been processed. * Relays are now loaded in chunks, with a progress bar updating to show how many have been loaded. * When loading the secondary search box is removed from view as the updates are happening asynchronously and bad things would happen if the load was interrupted by a second search. --- css/atlas.css | 4 +-- index.html | 2 +- js/collections/results.js | 49 +++++++++++++++++++++--------------- js/helpers.js | 9 +++++++ js/router.js | 64 ++++++++++++++++++++++++++--------------------- templates/search/do.html | 4 +-- 6 files changed, 78 insertions(+), 54 deletions(-)

diff --git a/css/atlas.css b/css/atlas.css index cdbec5e..41f799f 100644 --- a/css/atlas.css +++ b/css/atlas.css @@ -4,8 +4,8 @@ display: none; }

-.progress .bar { - width: 50%; +.progress-bar { + width: 100%; }

a.alleged { diff --git a/index.html b/index.html index 9ab3516..10aa909 100644 --- a/index.html +++ b/index.html @@ -151,7 +151,7 @@ </form> <h1>Relay Search</h1> <div class="progress progress-info progress-striped active"> - <div class="progress-bar"></div> + <div class="progress-bar">Rendering results...</div> </div> <div id="content"> <noscript> diff --git a/js/collections/results.js b/js/collections/results.js index da8a6a4..73ef819 100644 --- a/js/collections/results.js +++ b/js/collections/results.js @@ -41,29 +41,38 @@ define([ if (relays.length == 0) { error(0); return false; - } else if (relays.length > 500) { - relays = relays.slice(0, 500); + } else if (relays.length > 2000) { + relays = relays.slice(0, 2000); err = 4; } var lookedUpRelays = 0; - _.each(relays, function(relay) { - var lookedUp = function() { - lookedUpRelays++; - if (lookedUpRelays == relays.length) { - success(err, relaysPublished, bridgesPublished); - } - } - relay.lookup({ - success: function(){ - collection[options.add ? 'add' : 'reset'](relays, options); - lookedUp(); - }, - error: function() { - lookedUp(); - error(0); - } - }); - }); + var relayChunks = relays.chunk(600); + var chunkedLookup = function() { + _.each(relayChunks.pop(), function(relay) { + relay.lookup({ + success: function(){ + lookedUpRelays++; + }, + error: function() { + lookedUpRelays++; + error(0); + } + }); + }); + if (lookedUpRelays == relays.length) { + $('.progress-bar').width("100%"); + $('.progress-bar').html("Rendering results..."); + setTimeout(function() { + collection[options.add ? 'add' : 'reset'](relays, options); + success(err, relaysPublished, bridgesPublished); + }, 500); + } else { + $('.progress-bar').width((lookedUpRelays / relays.length * 100) + "%"); + $('.progress-bar').html(lookedUpRelays + " of " + relays.length + " loaded"); + setTimeout(chunkedLookup, 50); + } + } + chunkedLookup(); }).fail( function(jqXHR, textStatus, errorThrown) { if(jqXHR.statusText == "error") { diff --git a/js/helpers.js b/js/helpers.js index 214db67..457702b 100644 --- a/js/helpers.js +++ b/js/helpers.js @@ -278,3 +278,12 @@ window.onhashchange = function(){ $('.btn-navbar').click(); } } + +Object.defineProperty(Array.prototype, 'chunk', { + value: function(chunkSize) { + var R = []; + for (var i=0; i<this.length; i+=chunkSize) + R.push(this.slice(i,i+chunkSize)); + return R; + } +}); diff --git a/js/router.js b/js/router.js index 118c094..fed87ba 100644 --- a/js/router.js +++ b/js/router.js @@ -61,23 +61,24 @@ define([ // Empty aggregation query emptyAggregateSearch: function() { $(".breadcrumb").html("<li><a href="https://metrics.torproject.org/%5C%22%3EHome</a></li><li><a href="https://metrics.torproject.org/services.html%5C%22%3EServices</a></li><li><a href="#">Relay Search</a></li><li class="active">Error</li>"); - $("#secondary-search").show(); $("#secondary-search-query").val("");

+ $("#secondary-search").hide(); $("#content").hide(); $(".progress").show(); - doSearchView.error = 5; - doSearchView.renderError(); - $(".progress").hide(); - $("#content").show(); + aggregateSearchView.error = 5; + aggregateSearchView.renderError(); + $(".progress").hide(); + $("#secondary-search").show(); + $("#content").show();

}, // Perform a countries aggregation aggregateSearch: function(aType, query){ $(".breadcrumb").html("<li><a href="https://metrics.torproject.org/%5C%22%3EHome</a></li><li><a href="https://metrics.torproject.org/services.html%5C%22%3EServices</a></li><li><a href="#">Relay Search</a></li><li class="active">Aggregated search" + ((query) ? " for " + query : "") + "</li>"); - $("#secondary-search").show();

$("#content").hide(); + $("#secondary-search").hide(); $(".progress").show();

aggregateSearchView.collection.aType = (aType) ? aType : "all"; @@ -100,12 +101,14 @@ define([ aggregateSearchView.render(query); $("#search-title").text("Aggregated results" + ((query) ? " for " + query : "")); $(".progress").hide(); + $("#secondary-search").show(); $("#content").show(); }, error: function(err){ aggregateSearchView.error = err; aggregateSearchView.renderError(); $(".progress").hide(); + $("#secondary-search").show(); $("#content").show(); } }); @@ -114,8 +117,8 @@ define([ // Perform a search on Atlas doSearch: function(query){ $(".breadcrumb").html("<li><a href="https://metrics.torproject.org/%5C%22%3EHome</a></li><li><a href="https://metrics.torproject.org/services.html%5C%22%3EServices</a></li><li><a href="#">Relay Search</a></li><li class="active">Search for " + query + "</li>"); - $("#secondary-search").show();

+ $("#secondary-search").hide(); $("#content").hide(); $(".progress").show();

@@ -124,37 +127,40 @@ define([ doSearchView.renderError(); $(".progress").hide(); $("#content").show(); + $("#secondary-search").show(); } else { query = query.trim(); $("#secondary-search-query").val(query); doSearchView.collection.url = doSearchView.collection.baseurl + this.hashFingerprint(query); doSearchView.collection.lookup({ - success: function(err, relaysPublished, bridgesPublished){ - doSearchView.relays = doSearchView.collection.models; - // Redirect to the details page when there is exactly one - // search result. - if (doSearchView.relays.length == 1) { - document.location.replace("#details/" + - doSearchView.relays[0].fingerprint); - return; - } + success: function(err, relaysPublished, bridgesPublished){ + doSearchView.relays = doSearchView.collection.models; + // Redirect to the details page when there is exactly one + // search result. + if (doSearchView.relays.length == 1) { + document.location.replace("#details/" + + doSearchView.relays[0].fingerprint); + return; + } doSearchView.error = err; - doSearchView.relaysPublished = relaysPublished; - doSearchView.bridgesPublished = bridgesPublished; - doSearchView.render(query); - $("#search-title").text(query); - $(".progress").hide(); - $("#content").show(); - }, - - error: function(err){ + doSearchView.relaysPublished = relaysPublished; + doSearchView.bridgesPublished = bridgesPublished; + doSearchView.render(query); + $("#search-title").text(query); + $("#secondary-search").show(); + $(".progress").hide(); + $("#content").show(); + }, + + error: function(err){ doSearchView.error = err; doSearchView.renderError(); - $(".progress").hide(); - $("#content").show(); - } - }); + $(".progress").hide(); + $("#content").show(); + $("#secondary-search").show(); + } + }); } }, showTopRelays: function(){ diff --git a/templates/search/do.html b/templates/search/do.html index fb32fdb..3935182 100644 --- a/templates/search/do.html +++ b/templates/search/do.html @@ -39,8 +39,8 @@ <% if (error == 4) { %> <div class="alert alert-warning"> <strong>Too many matches!</strong><p>The current version of - Relay Search does not support a result set greater than 500 and only displays - the first 500 hits. This is due to some performance issues in rendering + Relay Search does not support a result set greater than 2000 and only displays + the first 2000 hits. This is due to some performance issues in rendering large results sets in JavaScript. Future versions will hopefully manage to overcome this issue.</p> <% if ( ! query.includes("running:true") ) { %><p>You may find it