Page MenuHomePhabricator
Files F15408990

D10600.id27430.diff
No OneTemporary
Actions

D10600.id27430.diff
View Options

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 @@
+<?php
+
+final class PhabricatorRepositoryBlameCache extends PhabricatorRepositoryDAO {
+
+ protected $pathId;
+ protected $commitId;
+ protected $blameDict;
+
+ public function getConfiguration() {
+ return array(
+ self::CONFIG_AUX_PHID => 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 @@
+<?php
+
+final class PhabricatorRepositoryCommitCacheParserWorker
+ extends PhabricatorRepositoryCommitParserWorker {
+
+ public function parseCommit(
+ PhabricatorRepository $repository,
+ PhabricatorRepositoryCommit $commit) {
+
+ $drequest = DiffusionRequest::newFromDictionary(
+ array(
+ 'user' => 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();
+ }
+ }
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 20, 2:35 AM (5 d, 18 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712399
Default Alt Text
D10600.id27430.diff (18 KB)

Event Timeline

Phabricator · Built by Phacility