From 6d0dbeb77f772d096aadd49f435b7a874a179454 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 20 May 2020 10:59:33 -0700 Subject: [PATCH] Use the changeset parse cache to cache suggestion changesets Summary: Ref T13513. Syntax highlighting is potentially expensive, and the changeset rendering pipeline can cache it. However, the cache is currently keyed ONLY by Differential changeset ID. Destroy the existing cache and rebuild it with a more standard cache key so it can be used in a more ad-hoc way by inline suggestion snippets. Test Plan: Used Darkconsole, saw cache hits and no more inline syntax highlighting for changesets with many inlines. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21280 --- .../autopatches/20200520.inline.01.remcache.sql | 1 + .../autopatches/20200520.inline.02.addcache.sql | 7 +++++++ .../parser/DifferentialChangesetParser.php | 14 ++++++-------- .../storage/DifferentialSchemaSpec.php | 9 +++++++-- .../diff/view/PHUIDiffInlineCommentDetailView.php | 13 +++++++++---- 5 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 resources/sql/autopatches/20200520.inline.01.remcache.sql create mode 100644 resources/sql/autopatches/20200520.inline.02.addcache.sql diff --git a/resources/sql/autopatches/20200520.inline.01.remcache.sql b/resources/sql/autopatches/20200520.inline.01.remcache.sql new file mode 100644 index 0000000000..356bf5a5a5 --- /dev/null +++ b/resources/sql/autopatches/20200520.inline.01.remcache.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_differential.differential_changeset_parse_cache; diff --git a/resources/sql/autopatches/20200520.inline.02.addcache.sql b/resources/sql/autopatches/20200520.inline.02.addcache.sql new file mode 100644 index 0000000000..7b9ed64aac --- /dev/null +++ b/resources/sql/autopatches/20200520.inline.02.addcache.sql @@ -0,0 +1,7 @@ +CREATE TABLE {$NAMESPACE}_differential.differential_changeset_parse_cache ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + cacheIndex BINARY(12) NOT NULL, + cache LONGBLOB NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + UNIQUE KEY `key_cacheIndex` (cacheIndex) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 35cdedbcc6..b7ea61b7a9 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -295,8 +295,6 @@ final class DifferentialChangesetParser extends Phobject { * By default, there is no render cache key and parsers do not use the cache. * This is appropriate for rarely-viewed changesets. * - * NOTE: Currently, this key must be a valid Differential Changeset ID. - * * @param string Key for identifying this changeset in the render cache. * @return this */ @@ -376,9 +374,9 @@ final class DifferentialChangesetParser extends Phobject { $conn_r = $changeset->establishConnection('r'); $data = queryfx_one( $conn_r, - 'SELECT * FROM %T WHERE id = %d', - $changeset->getTableName().'_parse_cache', - $render_cache_key); + 'SELECT * FROM %T WHERE cacheIndex = %s', + DifferentialChangeset::TABLE_CACHE, + PhabricatorHash::digestForIndex($render_cache_key)); if (!$data) { return false; @@ -480,12 +478,12 @@ final class DifferentialChangesetParser extends Phobject { try { queryfx( $conn_w, - 'INSERT INTO %T (id, cache, dateCreated) VALUES (%d, %B, %d) + 'INSERT INTO %T (cacheIndex, cache, dateCreated) VALUES (%s, %B, %d) ON DUPLICATE KEY UPDATE cache = VALUES(cache)', DifferentialChangeset::TABLE_CACHE, - $render_cache_key, + PhabricatorHash::digestForIndex($render_cache_key), $cache, - time()); + PhabricatorTime::getNow()); } catch (AphrontQueryException $ex) { // Ignore these exceptions. A common cause is that the cache is // larger than 'max_allowed_packet', in which case we're better off diff --git a/src/applications/differential/storage/DifferentialSchemaSpec.php b/src/applications/differential/storage/DifferentialSchemaSpec.php index 4f747af06c..d08a0b1e29 100644 --- a/src/applications/differential/storage/DifferentialSchemaSpec.php +++ b/src/applications/differential/storage/DifferentialSchemaSpec.php @@ -9,7 +9,8 @@ final class DifferentialSchemaSpec extends PhabricatorConfigSchemaSpec { id(new DifferentialRevision())->getApplicationName(), DifferentialChangeset::TABLE_CACHE, array( - 'id' => 'id', + 'id' => 'auto', + 'cacheIndex' => 'bytes12', 'cache' => 'bytes', 'dateCreated' => 'epoch', ), @@ -18,7 +19,11 @@ final class DifferentialSchemaSpec extends PhabricatorConfigSchemaSpec { 'columns' => array('id'), 'unique' => true, ), - 'dateCreated' => array( + 'key_cacheIndex' => array( + 'columns' => array('cacheIndex'), + 'unique' => true, + ), + 'key_created' => array( 'columns' => array('dateCreated'), ), ), diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php index ddf0dddf15..fdaa99ac7e 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -547,8 +547,6 @@ final class PHUIDiffInlineCommentDetailView $changeset->setFilename($context->getFilename()); - // TODO: This isn't cached! - $viewstate = new PhabricatorChangesetViewState(); $parser = id(new DifferentialChangesetParser()) @@ -556,6 +554,15 @@ final class PHUIDiffInlineCommentDetailView ->setViewstate($viewstate) ->setChangeset($changeset); + $fragment = $inline->getInlineCommentCacheFragment(); + if ($fragment !== null) { + $cache_key = sprintf( + '%s.suggestion-view(v1, %s)', + $fragment, + PhabricatorHash::digestForIndex($new_lines)); + $parser->setRenderCacheKey($cache_key); + } + $renderer = new DifferentialChangesetOneUpRenderer(); $renderer->setSimpleMode(true); @@ -572,6 +579,4 @@ final class PHUIDiffInlineCommentDetailView return $view; } - - } -- 2.51.2