From d0b6602e294d744be29d5825de159d112b560af5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 13 Sep 2011 11:00:17 -0700 Subject: [PATCH] Add an option to switch tokenizers to use "ondemand" instead of "preloaded" datasources Summary: The open source Phabricator has like 3,500 user accounts now and it takes a while to pull/render them. Add an option to switch to ondemand for large installs. I'll follow up with a patch at some point to address a couple of name things: - Denormalize last names into a keyed column (although this evidences some bias toward the western world). - Force all usernames to lowercase (sorry Girish, Makinde). Also this patch is so clean it's crazy. Didn't bother with other object types for now, I'm planning to dedicate a few days to Projects at some point and I'll flesh out some auxiliary features like this when I do that. Test Plan: Switched to ondemand, verified data was queried dynamically. Switched back, verified data was preloaded. Reviewers: jungejason, nh, tuomaspelkonen, aran Reviewed By: nh CC: aran, epriestley, nh Differential Revision: 923 --- conf/default.conf.php | 14 +++++ src/__celerity_resource_map__.php | 53 ++++++++++--------- ...torTypeaheadCommonDatasourceController.php | 14 ++++- .../tokenizer/AphrontFormTokenizerControl.php | 9 ++-- src/view/form/control/tokenizer/__init__.php | 1 + .../js/application/core/behavior-tokenizer.js | 8 ++- 6 files changed, 67 insertions(+), 32 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index 13230ce01b..9d434e87d3 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -402,6 +402,20 @@ return array( // You can enable traces for development to make it easier to debug problems. 'phabricator.show-stack-traces' => false, + // Tokenizers are UI controls which let the user select other users, email + // addresses, project names, etc., by typing the first few letters and having + // the control autocomplete from a list. They can load their data in two ways: + // either in a big chunk up front, or as the user types. By default, the data + // is loaded in a big chunk. This is simpler and performs better for small + // datasets. However, if you have a very large number of users or projects, + // (in the ballpark of more than a thousand), loading all that data may become + // slow enough that it's worthwhile to query on demand instead. This makes + // the typeahead slightly less responsive but overall performance will be much + // better if you have a ton of stuff. You can figure out which setting is + // best for your install by changing this setting and then playing with a + // user tokenizer (like the user selectors in Maniphest or Differential) and + // seeing which setting loads faster and feels better. + 'tokenizer.ondemand' => false, // -- Files ----------------------------------------------------------------- // diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 81b3f9670f..b448fd983c 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -332,7 +332,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-aphront-basic-tokenizer' => array( - 'uri' => '/res/5e183bd5/rsrc/js/application/core/behavior-tokenizer.js', + 'uri' => '/res/9be30797/rsrc/js/application/core/behavior-tokenizer.js', 'type' => 'js', 'requires' => array( @@ -340,8 +340,9 @@ celerity_register_resource_map(array( 1 => 'javelin-typeahead', 2 => 'javelin-tokenizer', 3 => 'javelin-typeahead-preloaded-source', - 4 => 'javelin-dom', - 5 => 'javelin-stratcom', + 4 => 'javelin-typeahead-ondemand-source', + 5 => 'javelin-dom', + 6 => 'javelin-stratcom', ), 'disk' => '/rsrc/js/application/core/behavior-tokenizer.js', ), @@ -1418,6 +1419,22 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/3dbf4083/javelin.pkg.js', 'type' => 'js', ), + '4aa8c13f' => + array( + 'name' => 'typeahead.pkg.js', + 'symbols' => + array( + 0 => 'javelin-typeahead', + 1 => 'javelin-typeahead-normalizer', + 2 => 'javelin-typeahead-source', + 3 => 'javelin-typeahead-preloaded-source', + 4 => 'javelin-typeahead-ondemand-source', + 5 => 'javelin-tokenizer', + 6 => 'javelin-behavior-aphront-basic-tokenizer', + ), + 'uri' => '/res/pkg/4aa8c13f/typeahead.pkg.js', + 'type' => 'js', + ), '7bf96a66' => array( 'name' => 'differential.pkg.css', @@ -1465,22 +1482,6 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/982ad44b/differential.pkg.js', 'type' => 'js', ), - 'ac869011' => - array( - 'name' => 'typeahead.pkg.js', - 'symbols' => - array( - 0 => 'javelin-typeahead', - 1 => 'javelin-typeahead-normalizer', - 2 => 'javelin-typeahead-source', - 3 => 'javelin-typeahead-preloaded-source', - 4 => 'javelin-typeahead-ondemand-source', - 5 => 'javelin-tokenizer', - 6 => 'javelin-behavior-aphront-basic-tokenizer', - ), - 'uri' => '/res/pkg/ac869011/typeahead.pkg.js', - 'type' => 'js', - ), 'f6422902' => array( 'name' => 'core.pkg.css', @@ -1527,7 +1528,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => '7bf96a66', 'diffusion-commit-view-css' => '03ef179e', 'javelin-behavior' => '3dbf4083', - 'javelin-behavior-aphront-basic-tokenizer' => 'ac869011', + 'javelin-behavior-aphront-basic-tokenizer' => '4aa8c13f', 'javelin-behavior-aphront-form-disable-on-submit' => '95c67dcd', 'javelin-behavior-differential-diff-radios' => '982ad44b', 'javelin-behavior-differential-edit-inline-comments' => '982ad44b', @@ -1543,12 +1544,12 @@ celerity_register_resource_map(array( 'javelin-mask' => '95c67dcd', 'javelin-request' => '3dbf4083', 'javelin-stratcom' => '3dbf4083', - 'javelin-tokenizer' => 'ac869011', - 'javelin-typeahead' => 'ac869011', - 'javelin-typeahead-normalizer' => 'ac869011', - 'javelin-typeahead-ondemand-source' => 'ac869011', - 'javelin-typeahead-preloaded-source' => 'ac869011', - 'javelin-typeahead-source' => 'ac869011', + 'javelin-tokenizer' => '4aa8c13f', + 'javelin-typeahead' => '4aa8c13f', + 'javelin-typeahead-normalizer' => '4aa8c13f', + 'javelin-typeahead-ondemand-source' => '4aa8c13f', + 'javelin-typeahead-preloaded-source' => '4aa8c13f', + 'javelin-typeahead-source' => '4aa8c13f', 'javelin-uri' => '3dbf4083', 'javelin-util' => '3dbf4083', 'javelin-vector' => '3dbf4083', diff --git a/src/applications/typeahead/controller/common/PhabricatorTypeaheadCommonDatasourceController.php b/src/applications/typeahead/controller/common/PhabricatorTypeaheadCommonDatasourceController.php index d3eb951c1e..051c7122fc 100644 --- a/src/applications/typeahead/controller/common/PhabricatorTypeaheadCommonDatasourceController.php +++ b/src/applications/typeahead/controller/common/PhabricatorTypeaheadCommonDatasourceController.php @@ -25,6 +25,9 @@ class PhabricatorTypeaheadCommonDatasourceController public function processRequest() { + $request = $this->getRequest(); + $query = $request->getStr('q'); + $need_users = false; $need_all_users = false; $need_lists = false; @@ -70,7 +73,16 @@ class PhabricatorTypeaheadCommonDatasourceController } if ($need_users) { - $users = id(new PhabricatorUser())->loadAll(); + if ($query) { + // TODO: We probably need to split last names here. Workaround until + // we get that up and running is to not enable server-side datasources. + $users = id(new PhabricatorUser())->loadAllWhere( + '(userName LIKE %> OR realName LIKE %>)', + $query, + $query); + } else { + $users = id(new PhabricatorUser())->loadAll(); + } foreach ($users as $user) { if (!$need_all_users) { if ($user->getIsSystemAgent()) { diff --git a/src/view/form/control/tokenizer/AphrontFormTokenizerControl.php b/src/view/form/control/tokenizer/AphrontFormTokenizerControl.php index 623dedaa94..c45e768383 100644 --- a/src/view/form/control/tokenizer/AphrontFormTokenizerControl.php +++ b/src/view/form/control/tokenizer/AphrontFormTokenizerControl.php @@ -58,10 +58,11 @@ class AphrontFormTokenizerControl extends AphrontFormControl { if (!$this->disableBehavior) { Javelin::initBehavior('aphront-basic-tokenizer', array( - 'id' => $id, - 'src' => $this->datasource, - 'value' => $values, - 'limit' => $this->limit, + 'id' => $id, + 'src' => $this->datasource, + 'value' => $values, + 'limit' => $this->limit, + 'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'), )); } diff --git a/src/view/form/control/tokenizer/__init__.php b/src/view/form/control/tokenizer/__init__.php index 9decd685ab..58443572f9 100644 --- a/src/view/form/control/tokenizer/__init__.php +++ b/src/view/form/control/tokenizer/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'view/control/tokenizer'); phutil_require_module('phabricator', 'view/form/control/base'); diff --git a/webroot/rsrc/js/application/core/behavior-tokenizer.js b/webroot/rsrc/js/application/core/behavior-tokenizer.js index 69338a34f0..a33b6b8673 100644 --- a/webroot/rsrc/js/application/core/behavior-tokenizer.js +++ b/webroot/rsrc/js/application/core/behavior-tokenizer.js @@ -4,6 +4,7 @@ * javelin-typeahead * javelin-tokenizer * javelin-typeahead-preloaded-source + * javelin-typeahead-ondemand-source * javelin-dom * javelin-stratcom */ @@ -11,7 +12,12 @@ JX.behavior('aphront-basic-tokenizer', function(config) { var root = JX.$(config.id); - var datasource = new JX.TypeaheadPreloadedSource(config.src); + var datasource; + if (config.ondemand) { + datasource = new JX.TypeaheadOnDemandSource(config.src); + } else { + datasource = new JX.TypeaheadPreloadedSource(config.src); + } var typeahead = new JX.Typeahead( root,