diff --git a/resources/sql/autopatches/20140928.schema.blamecache.sql b/resources/sql/autopatches/20140928.schema.blamecache.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140928.schema.blamecache.sql @@ -0,0 +1,6 @@ +CREATE TABLE {$NAMESPACE}_repository.repository_blamecache ( + pathId int(10) unsigned NOT NULL, + commitId int(10) unsigned NOT NULL, + blameDict mediumblob NOT NULL, + PRIMARY KEY (pathId,commitId) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2258,8 +2258,10 @@ 'PhabricatorRepositoryArcanistProjectPHIDType' => 'applications/repository/phid/PhabricatorRepositoryArcanistProjectPHIDType.php', 'PhabricatorRepositoryArcanistProjectQuery' => 'applications/repository/query/PhabricatorRepositoryArcanistProjectQuery.php', 'PhabricatorRepositoryAuditRequest' => 'applications/repository/storage/PhabricatorRepositoryAuditRequest.php', + 'PhabricatorRepositoryBlameCache' => 'applications/repository/storage/PhabricatorRepositoryBlameCache.php', 'PhabricatorRepositoryBranch' => 'applications/repository/storage/PhabricatorRepositoryBranch.php', 'PhabricatorRepositoryCommit' => 'applications/repository/storage/PhabricatorRepositoryCommit.php', + 'PhabricatorRepositoryCommitCacheParserWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitCacheParserWorker.php', 'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php', 'PhabricatorRepositoryCommitData' => 'applications/repository/storage/PhabricatorRepositoryCommitData.php', 'PhabricatorRepositoryCommitHeraldWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php', @@ -5504,6 +5506,7 @@ 'PhabricatorRepositoryDAO', 'PhabricatorPolicyInterface', ), + 'PhabricatorRepositoryBlameCache' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryBranch' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryCommit' => array( 'PhabricatorRepositoryDAO', @@ -5517,6 +5520,7 @@ 'PhabricatorCustomFieldInterface', 'PhabricatorApplicationTransactionInterface', ), + 'PhabricatorRepositoryCommitCacheParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryCommitHeraldWorker' => 'PhabricatorRepositoryCommitParserWorker', diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -494,6 +494,7 @@ } $display = array(); + $commit_phids = array(); $line_number = 1; $last_rev = null; @@ -536,6 +537,7 @@ $display_line['epoch'] = idx($blame, 'epoch'); $display_line['color'] = $color; $display_line['commit'] = $rev; + $commit_phids[] = idx($blame, 'commitPHID'); $author_phid = idx($blame, 'authorPHID'); if ($author_phid && $handles[$author_phid]) { @@ -568,38 +570,39 @@ $request = $this->getRequest(); $viewer = $request->getUser(); - $commits = array_filter(ipull($display, 'commit')); - if ($commits) { + // If we have commitIDs try to find corresponding differential revisions + if ($commit_phids) { + $commit_phids = array_unique($commit_phids); $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) ->withRepository($drequest->getRepository()) - ->withIdentifiers($commits) + ->withPHIDs($commit_phids) ->execute(); $commits = mpull($commits, null, 'getCommitIdentifier'); - } - $revision_ids = id(new DifferentialRevision()) - ->loadIDsByCommitPHIDs(mpull($commits, 'getPHID')); - $revisions = array(); - if ($revision_ids) { - $revisions = id(new DifferentialRevisionQuery()) - ->setViewer($viewer) - ->withIDs($revision_ids) - ->execute(); - } + $revision_ids = id(new DifferentialRevision()) + ->loadIDsByCommitPHIDs($commit_phids); + $revisions = array(); + if ($revision_ids) { + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withIDs($revision_ids) + ->execute(); + } - $phids = array(); - foreach ($commits as $commit) { - if ($commit->getAuthorPHID()) { - $phids[] = $commit->getAuthorPHID(); + $phids = array(); + foreach ($commits as $commit) { + if ($commit->getAuthorPHID()) { + $phids[] = $commit->getAuthorPHID(); + } } - } - foreach ($revisions as $revision) { - if ($revision->getAuthorPHID()) { - $phids[] = $revision->getAuthorPHID(); + foreach ($revisions as $revision) { + if ($revision->getAuthorPHID()) { + $phids[] = $revision->getAuthorPHID(); + } } + $handles = $this->loadViewerHandles($phids); } - $handles = $this->loadViewerHandles($phids); Javelin::initBehavior('phabricator-oncopy', array()); diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -369,8 +369,7 @@ } if ($refs) { - $callsigns = ipull($refs, 'callsign'); - + $callsigns = array_unique(ipull($refs, 'callsign')); $repos = id(new PhabricatorRepositoryQuery()) ->setViewer($this->getViewer()) ->withIdentifiers($callsigns); diff --git a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php --- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php @@ -11,6 +11,7 @@ private $needsBlame; private $fileContent; private $viewer; + private $cachedBlameInfo; final public static function newFromDiffusionRequest( DiffusionRequest $request) { @@ -39,13 +40,76 @@ } final public function loadFileContent() { - return $this->executeQuery(); + if ($this->needsBlame) { + $this->loadBlameCache(); + } + $content = $this->executeQuery(); + return $content; } final public function getRawData() { return $this->fileContent->getCorpus(); } + + /** Blame Cache Functions */ + + // Returns array(pathid, commitId) for the queried file + private function getBlameCacheKey() { + $path_id = id( + new DiffusionPathIDQuery( + array($this->getRequest()->getPath()) + ))->loadPathIDs(); + $path_id = array_shift($path_id); + + // FIXME: The code in the diffusion.historyquery function should + // probably be moved to a separate Diffusion Query Class + // instead of sitting in the API directly + $latest_commit = DiffusionQuery::callConduitWithDiffusionRequest( + $this->getRequest()->getUser(), + $this->getRequest(), + 'diffusion.historyquery', + array( + 'commit' => $this->getRequest()->getCommit(), + 'path' => $this->getRequest()->getPath(), + 'callsign' => $this->getRequest()->getRepository()->getCallsign(), + 'offset' => 0, + 'limit' => 1, + )); + + if (!isset($latest_commit['pathChanges']) || + !isset($latest_commit['pathChanges'][0])) { + throw new Exception('Unable to find the latest commit for path'); + } + $commit_id = $latest_commit['pathChanges'][0]['commitData']['commitID']; + + return array($path_id, $commit_id); + } + + private function loadBlameCache() { + try { + $key = $this->getBlameCacheKey(); + $cache_data = + PhabricatorRepositoryBlameCache::loadFromCache($key[0], $key[1]); + if ($cache_data) { + $this->cachedBlameInfo = $cache_data->getBlameDict(); + } + } catch (Exception $e) { + // Move on if the cache fails... + phlog($e); + } + } + + private function writeBlameCache($blame_dict) { + $key = $this->getBlameCacheKey(); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + PhabricatorRepositoryBlameCache::saveToCache($key[0], $key[1], $blame_dict); + unset($unguarded); + + return true; + } + /** * Pretty hairy function. If getNeedsBlame is false, this returns * @@ -72,58 +136,69 @@ $line_rev_dict = array(); $blame_dict = array(); - if (!$this->getNeedsBlame()) { + if (!$this->getNeedsBlame(true)) { $text_list = explode("\n", $raw_data); } else if ($raw_data != '') { - $lines = array(); - foreach (explode("\n", $raw_data) as $k => $line) { - $lines[$k] = $this->tokenizeLine($line); - - list($rev_id, $author, $text) = $lines[$k]; - $text_list[$k] = $text; - $line_rev_dict[$k] = $rev_id; - } + // Check if we have a cached blame + if ($this->cachedBlameInfo) { + $text_list = explode("\n", $raw_data); + $cache_data = $this->cachedBlameInfo; + + $commits = id(new DiffusionCommitQuery()) + ->setViewer($this->getViewer()) + ->withDefaultRepository($this->getRequest()->getRepository()) + ->withIDs(array_unique($cache_data)) + ->needCommitData(true) + ->execute(); + + // Rebuild line_rev_dict from cache + foreach ($cache_data as $k => $commit_id) { + $line_rev_dict[$k] = $commits[$commit_id]->getCommitIdentifier(); + } + } else { + $lines = array(); + foreach (explode("\n", $raw_data) as $k => $line) { + $lines[$k] = $this->tokenizeLine($line); + + list($rev_id, $author, $text) = $lines[$k]; + $text_list[$k] = $text; + $line_rev_dict[$k] = $rev_id; + } - $line_rev_dict = $this->processRevList($line_rev_dict); + $line_rev_dict = $this->processRevList($line_rev_dict); - foreach ($lines as $k => $line) { - list($rev_id, $author, $text) = $line; - $rev_id = $line_rev_dict[$k]; + $commits = id(new DiffusionCommitQuery()) + ->setViewer($this->getViewer()) + ->withDefaultRepository($this->getRequest()->getRepository()) + ->withIdentifiers(array_unique($line_rev_dict)) + ->needCommitData(true) + ->execute(); - if (!isset($blame_dict[$rev_id])) { - $blame_dict[$rev_id]['author'] = $author; + // Write results to cache + $cache_data = array(); + $commit_identifiers = mpull($commits, 'getID', 'getCommitIdentifier'); + foreach ($line_rev_dict as $k => $line) { + $cache_data[$k] = $commit_identifiers[$line]; } - } - - $repository = $this->getRequest()->getRepository(); - $commits = id(new DiffusionCommitQuery()) - ->setViewer($this->getViewer()) - ->withDefaultRepository($repository) - ->withIdentifiers(array_unique($line_rev_dict)) - ->execute(); + $this->writeBlameCache($cache_data); + } foreach ($commits as $commit) { $blame_dict[$commit->getCommitIdentifier()]['epoch'] = $commit->getEpoch(); - } - if ($commits) { - $commits_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere( - 'commitID IN (%Ls)', - mpull($commits, 'getID')); - - foreach ($commits_data as $data) { - $author_phid = $data->getCommitDetail('authorPHID'); - if (!$author_phid) { - continue; - } - $commit = $commits[$data->getCommitID()]; - $commit_identifier = $commit->getCommitIdentifier(); - $blame_dict[$commit_identifier]['authorPHID'] = $author_phid; + $blame_dict[$commit->getCommitIdentifier()]['commitPHID'] = + $commit->getPHID(); + + $author_phid = $commit->getCommitData()->getCommitDetail('authorPHID'); + if ($author_phid) { + $blame_dict[$commit->getCommitIdentifier()]['author'] = $author_phid; + } else { + $blame_dict[$commit->getCommitIdentifier()]['author'] = + $commit->getCommitData()->getCommitDetail('authorName'); } } - } return array($text_list, $line_rev_dict, $blame_dict); @@ -136,8 +211,16 @@ return $this; } - public function getNeedsBlame() { - return $this->needsBlame; + public function getNeedsBlame($ignore_cache = false) { + if ($ignore_cache) { + return $this->needsBlame; + } else { + if (!$this->cachedBlameInfo) { + return $this->needsBlame; + } + } + + return false; } public function setViewer(PhabricatorUser $user) { diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php --- a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php @@ -57,6 +57,10 @@ 'help' => pht('Reparse changes.'), ), array( + 'name' => 'cache', + 'help' => 'Reparse cache information', + ), + array( 'name' => 'herald', 'help' => pht( 'Reevaluate Herald rules (may send huge amounts of email!)'), @@ -95,6 +99,7 @@ $all_from_repo = $args->getArg('all'); $reparse_message = $args->getArg('message'); $reparse_change = $args->getArg('change'); + $reparse_cache = $args->getArg('cache'); $reparse_herald = $args->getArg('herald'); $reparse_owners = $args->getArg('owners'); $reparse_what = $args->getArg('revision'); @@ -118,11 +123,11 @@ $commits)); } - if (!$reparse_message && !$reparse_change && !$reparse_herald && - !$reparse_owners) { + if (!$reparse_message && !$reparse_change && !$reparse_cache && + !$reparse_herald && !$reparse_owners) { throw new PhutilArgumentUsageException( pht('Specify what information to reparse with --message, --change, '. - '--herald, and/or --owners')); + '--cache, --herald, and/or --owners')); } $min_timestamp = false; @@ -268,6 +273,10 @@ break; } + if ($reparse_cache) { + $classes[] = 'PhabricatorRepositoryCommitCacheParserWorker'; + } + if ($reparse_herald) { $classes[] = 'PhabricatorRepositoryCommitHeraldWorker'; } diff --git a/src/applications/repository/storage/PhabricatorRepositoryBlameCache.php b/src/applications/repository/storage/PhabricatorRepositoryBlameCache.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/storage/PhabricatorRepositoryBlameCache.php @@ -0,0 +1,53 @@ + false, + self::CONFIG_TIMESTAMPS => false, + self::CONFIG_BINARY => array( + 'blameDict' => true, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'id' => null, + 'pathId' => 'uint32', + 'commitId' => 'uint32', + 'blameDict' => 'bytes', + ), + self::CONFIG_KEY_SCHEMA => array( + 'PRIMARY' => array( + 'columns' => array('pathId', 'commitId'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + + public function willWriteData(array &$data) { + $data['blameDict'] = gzcompress(implode(' ', $data['blameDict'])); + parent::willWriteData($data); + } + + public function willReadData(array &$data) { + $data['blameDict'] = explode(' ', gzuncompress($data['blameDict'])); + parent::willReadData($data); + } + + public static function saveToCache($path_id, $commit_id, $blame_dict) { + return id(new PhabricatorRepositoryBlameCache()) + ->setPathId($path_id) + ->setCommitId($commit_id) + ->setBlameDict($blame_dict) + ->replace(); + } + + public static function loadFromCache($path_id, $commit_id) { + return id(new PhabricatorRepositoryBlameCache()) + ->loadOneWhere('pathId = %d AND commitId = %d', $path_id, $commit_id); + } +} diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitCacheParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitCacheParserWorker.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitCacheParserWorker.php @@ -0,0 +1,46 @@ + PhabricatorUser::getOmnipotentUser(), + 'initFromConduit' => false, + 'repository' => $repository, + 'commit' => $commit->getCommitIdentifier(), + )); + + $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( + $drequest); + + $changes = $change_query->loadChanges(); + + foreach ($changes as $change) { + $filetype = $change->getFileType(); + if ($filetype == DifferentialChangeType::FILE_TEXT || + $filetype == DifferentialChangeType::FILE_NORMAL) { + + $drequest = DiffusionRequest::newFromDictionary( + array( + 'user' => PhabricatorUser::getOmnipotentUser(), + 'initFromConduit' => false, + 'repository' => $repository, + 'commit' => $commit->getCommitIdentifier(), + 'path' => $change->getPath(), + )); + + $blame_request = + DiffusionFileContentQuery::newFromDiffusionRequest($drequest) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setNeedsBlame(true); + $blame_request->loadFileContent(); + $blame_request->getBlameData(); + } + } + } +}