D10600.id25539.diff
No OneTemporary
Actions

Size

20 KB

Referenced Files

None

Subscribers

None

D10600.id25539.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/scripts/repository/reparse.php b/scripts/repository/reparse.php
	--- a/scripts/repository/reparse.php
	+++ b/scripts/repository/reparse.php
	@@ -65,6 +65,10 @@
	'help' => 'Reparse changes.',
	),
	array(
	+ 'name' => 'cache',
	+ 'help' => 'Reparse cache information',
	+ ),
	+ array(
	'name' => 'herald',
	'help' => 'Reevaluate Herald rules (may send huge amounts of email!)',
	),
	@@ -94,6 +98,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_harbormaster = $args->getArg('harbormaster');
	@@ -114,8 +119,8 @@
	"Commit(s) to reparse: ".$commits);
	}

	-if (!$reparse_message && !$reparse_change && !$reparse_herald &&
	- !$reparse_owners && !$reparse_harbormaster) {
	+if (!$reparse_message && !$reparse_change && !$reparse_cache &&
	+ !$reparse_herald && !$reparse_owners && !$reparse_harbormaster) {
	usage('Specify what information to reparse with --message, --change, '.
	'--herald, --harbormaster, and/or --owners');
	}
	@@ -251,6 +256,10 @@
	break;
	}

	+ if ($reparse_cache) {
	+ $classes[] = 'PhabricatorRepositoryCommitCacheParserWorker';
	+ }
	+
	if ($reparse_herald) {
	$classes[] = 'PhabricatorRepositoryCommitHeraldWorker';
	}
	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
	@@ -2077,8 +2077,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',
	@@ -5046,6 +5048,7 @@
	'PhabricatorRepositoryDAO',
	'PhabricatorPolicyInterface',
	),
	+ 'PhabricatorRepositoryBlameCache' => 'PhabricatorRepositoryDAO',
	'PhabricatorRepositoryBranch' => 'PhabricatorRepositoryDAO',
	'PhabricatorRepositoryCommit' => array(
	'PhabricatorRepositoryDAO',
	@@ -5058,6 +5061,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
	@@ -496,6 +496,7 @@
	}

	$display = array();
	+ $commit_phids = array();

	$line_number = 1;
	$last_rev = null;
	@@ -538,6 +539,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]) {
	@@ -570,38 +572,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
	@@ -361,7 +361,7 @@
	}

	if ($refs) {
	- $callsigns = ipull($refs, 'callsign');
	+ $callsigns = array_unique(ipull($refs, 'callsign'));
	$repos = id(new PhabricatorRepositoryQuery())
	->setViewer($this->getViewer())
	->withCallsigns($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,59 @@
	}

	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 */
	+
	+ abstract protected function getLatestCommitHashForPath();
	+
	+ // 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);
	+
	+ $latest_commit = $this->getLatestCommitHashForPath();
	+
	+ // Get the commitId from the Hash
	+ $commit_id = id(id(new PhabricatorRepositoryCommit())
	+ ->attachRepository($this->getRequest()->getRepository())
	+ ->loadOneWhere('commitIdentifier = %s', $latest_commit))->getId();
	+
	+ return array($path_id, $commit_id);
	+ }
	+
	+ private function loadBlameCache() {
	+ $key = $this->getBlameCacheKey();
	+ $cache_data =
	+ PhabricatorRepositoryBlameCache::loadFromCache($key[0], $key[1]);
	+ if ($cache_data) {
	+ $this->cachedBlameInfo = $cache_data->getBlameDict();
	+ }
	+ }
	+
	+ 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 +119,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 +194,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/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
	--- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
	+++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
	@@ -31,6 +31,14 @@
	return $file_content;
	}

	+ protected function getLatestCommitHashForPath() {
	+ $latest_commit = $this->getRequest()->getRepository()
	+ ->execxLocalCommand('log -n1 --format="%%H" %s -- %s',
	+ $this->getRequest()->getCommit(),
	+ $this->getRequest()->getPath());
	+ return trim(array_shift($latest_commit));
	+ }
	+
	protected function tokenizeLine($line) {
	return self::match($line);
	}
	diff --git a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
	--- a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
	+++ b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
	@@ -34,6 +34,16 @@
	return $file_content;
	}

	+ protected function getLatestCommitHashForPath() {
	+ // FIXME: Adjust this git command for mercurial
	+ /* $latest_commit = $this->getRequest()->getRepository()
	+ ->execxLocalCommand('log -n1 --format="%%H" %s -- %s',
	+ $this->getRequest()->getCommit(),
	+ $this->getRequest()->getPath());
	+ return trim(array_shift($latest_commit));*/
	+ throw new Exception('Not implemented yet');
	+ }
	+
	protected function tokenizeLine($line) {
	$matches = null;

	diff --git a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
	--- a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
	+++ b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
	@@ -40,6 +40,16 @@
	return $file_content;
	}

	+ protected function getLatestCommitHashForPath() {
	+ // FIXME: Adjust this git command for svn
	+ /* $latest_commit = $this->getRequest()->getRepository()
	+ ->execxLocalCommand('log -n1 --format="%%H" %s -- %s',
	+ $this->getRequest()->getCommit(),
	+ $this->getRequest()->getPath());
	+ return trim(array_shift($latest_commit));*/
	+ throw new Exception('Not implemented yet');
	+ }
	+
	protected function tokenizeLine($line) {
	// sample line:
	// 347498 yliu function print();
	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,55 @@
	+<?php
	+
	+final class PhabricatorRepositoryBlameCache extends PhabricatorRepositoryDAO {
	+
	+ protected $pathId;
	+ protected $commitId;
	+ protected $blameDict;
	+
	+ public function getConfiguration() {
	+ // FIXME: How do i configure this to not have an id column?
	+ return array(
	+ self::CONFIG_AUX_PHID => false,
	+ self::CONFIG_TIMESTAMPS => false,
	+// self::CONFIG_IDS => self::IDS_MANUAL,
	+ self::CONFIG_BINARY => array(
	+ 'blameDict' => true,
	+ ),
	+ self::CONFIG_COLUMN_SCHEMA => array(
	+// 'id' => null,
	+ 'pathId' => 'uint32',
	+ 'commitId' => 'uint32',
	+ 'blameDict' => 'mediumblob',
	+ ),
	+ 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: Wed, Mar 19, 10:34 PM (4 d, 22 h ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 7654852
Default Alt Text: D10600.id25539.diff (20 KB)

D10600.id25539.diffNo OneTemporaryActions

D10600.id25539.diffView Options

File Metadata

Event Timeline

D10600.id25539.diff
No OneTemporary
Actions

D10600.id25539.diff
View Options