1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-09 16:32:39 +01:00

Remove tokenizer.ondemand, and always load on demand

Summary:
Ref T4420. Tokenizers currently operate in "preload" or "ondemand" modes. In the former mode, which is default, they'll try to load the entire result list when a page loads.

The theory here was that this would slightly improve the experience for small installs, and once they got big enough they could switch to "ondemand". In practice, several issues have arisen:

  - We generally don't have a good mechanism for telling installs that they should tweak perf config -- `metamta.send-immediately` is the canonical example here. Some large installs are probably affected negatively by not knowing to change this setting, and having settings like this is generally annoying.
  - We have way way too much config now.
  - With the advent of ApplicationSearch, pages like Maniphest make many redundant loads to prefill sources like projects. Most of the time, this data is not used. It's far simpler to switch everything to ondemand than try to deal with this, and dealing with this would mean creating two very complex divergent pathways in the codebase for a mostly theoretical performance benefit which only impacts tiny installs.
  - We've been using `tokenizer.ondemand` forever on `secure.phabricator.com` since we have many thousands of user accounts, and it doesn't seem sluggish and works properly.

Removing this config is an easy fix which makes the codebase simpler.

I've retained the ability to use preloaded sources, since they may make sense in some cases (in at least one case -- task priorities -- adding a static source pathway might make sense), and they're part of Javelin itself. However, the code will no longer ever go down that pathway.

Test Plan: Used `secure.phabricator.com` for years with this setting enabled.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4420

Differential Revision: https://secure.phabricator.com/D8232
This commit is contained in:
epriestley 2014-02-14 10:24:40 -08:00
parent 87012d60f1
commit a0262c0b4f
9 changed files with 27 additions and 66 deletions

View file

@ -638,21 +638,6 @@ return array(
'https' => true, 'https' => true,
), ),
// 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,
// By default, Phabricator includes some silly nonsense in the UI, such as // By default, Phabricator includes some silly nonsense in the UI, such as
// a submit button called "Clowncopterize" in Differential and a call to // a submit button called "Clowncopterize" in Differential and a call to
// "Leap Into Action". If you'd prefer more traditional UI strings like // "Leap Into Action". If you'd prefer more traditional UI strings like

View file

@ -8,7 +8,7 @@ return array(
'names' => 'names' =>
array( array(
'core.pkg.css' => '044c2f0c', 'core.pkg.css' => '044c2f0c',
'core.pkg.js' => 'ba6e232d', 'core.pkg.js' => '0dc59a05',
'darkconsole.pkg.js' => 'ca8671ce', 'darkconsole.pkg.js' => 'ca8671ce',
'differential.pkg.css' => '6aef439e', 'differential.pkg.css' => '6aef439e',
'differential.pkg.js' => '322ea941', 'differential.pkg.js' => '322ea941',
@ -431,7 +431,7 @@ return array(
'rsrc/js/core/KeyboardShortcutManager.js' => 'ad7a69ca', 'rsrc/js/core/KeyboardShortcutManager.js' => 'ad7a69ca',
'rsrc/js/core/MultirowRowManager.js' => 'e7076916', 'rsrc/js/core/MultirowRowManager.js' => 'e7076916',
'rsrc/js/core/Notification.js' => '95944043', 'rsrc/js/core/Notification.js' => '95944043',
'rsrc/js/core/Prefab.js' => '1e644329', 'rsrc/js/core/Prefab.js' => '979f864d',
'rsrc/js/core/ShapedRequest.js' => 'dfa181a4', 'rsrc/js/core/ShapedRequest.js' => 'dfa181a4',
'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc', 'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc',
'rsrc/js/core/ToolTip.js' => '0a81ea29', 'rsrc/js/core/ToolTip.js' => '0a81ea29',
@ -701,7 +701,7 @@ return array(
'phabricator-object-list-view-css' => '1a1ea560', 'phabricator-object-list-view-css' => '1a1ea560',
'phabricator-object-selector-css' => '029a133d', 'phabricator-object-selector-css' => '029a133d',
'phabricator-phtize' => 'd254d646', 'phabricator-phtize' => 'd254d646',
'phabricator-prefab' => '1e644329', 'phabricator-prefab' => '979f864d',
'phabricator-profile-css' => '3a7e04ca', 'phabricator-profile-css' => '3a7e04ca',
'phabricator-project-tag-css' => '095c9404', 'phabricator-project-tag-css' => '095c9404',
'phabricator-remarkup-css' => 'ca7f2265', 'phabricator-remarkup-css' => 'ca7f2265',
@ -928,19 +928,6 @@ return array(
5 => 'phabricator-drag-and-drop-file-upload', 5 => 'phabricator-drag-and-drop-file-upload',
6 => 'phabricator-draggable-list', 6 => 'phabricator-draggable-list',
), ),
'1e644329' =>
array(
0 => 'javelin-install',
1 => 'javelin-util',
2 => 'javelin-dom',
3 => 'javelin-typeahead',
4 => 'javelin-tokenizer',
5 => 'javelin-typeahead-preloaded-source',
6 => 'javelin-typeahead-ondemand-source',
7 => 'javelin-dom',
8 => 'javelin-stratcom',
9 => 'javelin-util',
),
'1f595fb0' => '1f595fb0' =>
array( array(
0 => 'javelin-install', 0 => 'javelin-install',
@ -1411,6 +1398,19 @@ return array(
2 => 'javelin-view-visitor', 2 => 'javelin-view-visitor',
3 => 'javelin-util', 3 => 'javelin-util',
), ),
'979f864d' =>
array(
0 => 'javelin-install',
1 => 'javelin-util',
2 => 'javelin-dom',
3 => 'javelin-typeahead',
4 => 'javelin-tokenizer',
5 => 'javelin-typeahead-preloaded-source',
6 => 'javelin-typeahead-ondemand-source',
7 => 'javelin-dom',
8 => 'javelin-stratcom',
9 => 'javelin-util',
),
'9b9197be' => '9b9197be' =>
array( array(
0 => 'javelin-behavior', 0 => 'javelin-behavior',

View file

@ -178,6 +178,8 @@ final class PhabricatorSetupCheckExtraConfig extends PhabricatorSetupCheck {
'Mail is now always delivered by the daemons.'), 'Mail is now always delivered by the daemons.'),
'auth.sessions.conduit' => $session_reason, 'auth.sessions.conduit' => $session_reason,
'auth.sessions.web' => $session_reason, 'auth.sessions.web' => $session_reason,
'tokenizer.ondemand' => pht(
'Phabricator now manages typeahead strategies automatically.'),
); );
return $ancient_config; return $ancient_config;

View file

@ -137,32 +137,6 @@ final class PhabricatorCoreConfigOptions
" %s", $path)) " %s", $path))
->addExample('/usr/local/bin', pht('Add One Path')) ->addExample('/usr/local/bin', pht('Add One Path'))
->addExample("/usr/bin\n/usr/local/bin", pht('Add Multiple Paths')), ->addExample("/usr/bin\n/usr/local/bin", pht('Add Multiple Paths')),
$this->newOption('tokenizer.ondemand', 'bool', false)
->setBoolOptions(
array(
pht("Query on demand"),
pht("Query on page load"),
))
->setSummary(
pht("Query for tokenizer fields on demand."))
->setDescription(
pht(
"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.")),
$this->newOption('config.lock', 'set', array()) $this->newOption('config.lock', 'set', array())
->setLocked(true) ->setLocked(true)
->setDescription(pht('Additional configuration options to lock.')), ->setDescription(pht('Additional configuration options to lock.')),

View file

@ -121,7 +121,6 @@ final class DifferentialAddCommentView extends AphrontView {
'src' => '/typeahead/common/usersorprojects/', 'src' => '/typeahead/common/usersorprojects/',
'value' => $this->reviewers, 'value' => $this->reviewers,
'row' => 'add-reviewers', 'row' => 'add-reviewers',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'labels' => $add_reviewers_labels, 'labels' => $add_reviewers_labels,
'placeholder' => pht('Type a user or project name...'), 'placeholder' => pht('Type a user or project name...'),
), ),
@ -130,7 +129,6 @@ final class DifferentialAddCommentView extends AphrontView {
'src' => '/typeahead/common/mailable/', 'src' => '/typeahead/common/mailable/',
'value' => $this->ccs, 'value' => $this->ccs,
'row' => 'add-ccs', 'row' => 'add-ccs',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a user or mailing list...'), 'placeholder' => pht('Type a user or mailing list...'),
), ),
), ),

View file

@ -780,14 +780,12 @@ final class DiffusionCommitController extends DiffusionController {
'actions' => array('add_auditors' => 1), 'actions' => array('add_auditors' => 1),
'src' => '/typeahead/common/users/', 'src' => '/typeahead/common/users/',
'row' => 'add-auditors', 'row' => 'add-auditors',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a user name...'), 'placeholder' => pht('Type a user name...'),
), ),
'add-ccs-tokenizer' => array( 'add-ccs-tokenizer' => array(
'actions' => array('add_ccs' => 1), 'actions' => array('add_ccs' => 1),
'src' => '/typeahead/common/mailable/', 'src' => '/typeahead/common/mailable/',
'row' => 'add-ccs', 'row' => 'add-ccs',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a user or mailing list...'), 'placeholder' => pht('Type a user or mailing list...'),
), ),
), ),

View file

@ -308,7 +308,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
ManiphestTransaction::TYPE_PROJECTS => array( ManiphestTransaction::TYPE_PROJECTS => array(
'id' => 'projects-tokenizer', 'id' => 'projects-tokenizer',
'src' => '/typeahead/common/projects/', 'src' => '/typeahead/common/projects/',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a project name...'), 'placeholder' => pht('Type a project name...'),
), ),
ManiphestTransaction::TYPE_OWNER => array( ManiphestTransaction::TYPE_OWNER => array(
@ -316,13 +315,11 @@ final class ManiphestTaskDetailController extends ManiphestController {
'src' => '/typeahead/common/users/', 'src' => '/typeahead/common/users/',
'value' => $default_claim, 'value' => $default_claim,
'limit' => 1, 'limit' => 1,
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a user name...'), 'placeholder' => pht('Type a user name...'),
), ),
ManiphestTransaction::TYPE_CCS => array( ManiphestTransaction::TYPE_CCS => array(
'id' => 'cc-tokenizer', 'id' => 'cc-tokenizer',
'src' => '/typeahead/common/mailable/', 'src' => '/typeahead/common/mailable/',
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'placeholder' => pht('Type a user or mailing list...'), 'placeholder' => pht('Type a user or mailing list...'),
), ),
); );

View file

@ -64,7 +64,6 @@ final class AphrontFormTokenizerControl extends AphrontFormControl {
'value' => mpull($values, 'getFullName', 'getPHID'), 'value' => mpull($values, 'getFullName', 'getPHID'),
'icons' => mpull($values, 'getTypeIcon', 'getPHID'), 'icons' => mpull($values, 'getTypeIcon', 'getPHID'),
'limit' => $this->limit, 'limit' => $this->limit,
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'username' => $username, 'username' => $username,
'placeholder' => $this->placeholder, 'placeholder' => $this->placeholder,
)); ));

View file

@ -58,7 +58,15 @@ JX.install('Prefab', {
} }
var datasource; var datasource;
if (config.ondemand) {
// Default to an ondemand source if no alternate configuration is
// provided.
var ondemand = true;
if ('ondemand' in config) {
ondemand = config.ondemand;
}
if (ondemand) {
datasource = new JX.TypeaheadOnDemandSource(config.src); datasource = new JX.TypeaheadOnDemandSource(config.src);
} else { } else {
datasource = new JX.TypeaheadPreloadedSource(config.src); datasource = new JX.TypeaheadPreloadedSource(config.src);