From 51ce3b4f555cd374782a88d1f6c175ab2bf75742 Mon Sep 17 00:00:00 2001 From: Chuck Hagenbuch Date: Sun, 29 Aug 2010 21:19:54 -0400 Subject: [PATCH] First set of refactorings to lazy-load some information and optimize serialized objects. Previously we had no filter on serializing these objects for caching, and a lot of them have recursive references. Not serializing those saves a lot of time and memory. This probably has some bugs left, definitely needs to be updated for the CVS driver, and certainly can be improved further. --- framework/Vcs/lib/Horde/Vcs.php | 102 +++++++++++++++++++++++++++--------- framework/Vcs/lib/Horde/Vcs/Git.php | 69 ++++++++++++++---------- framework/Vcs/lib/Horde/Vcs/Svn.php | 72 +++++++++++++++---------- 3 files changed, 162 insertions(+), 81 deletions(-) diff --git a/framework/Vcs/lib/Horde/Vcs.php b/framework/Vcs/lib/Horde/Vcs.php index c29a79b6f..ed93d2bd3 100644 --- a/framework/Vcs/lib/Horde/Vcs.php +++ b/framework/Vcs/lib/Horde/Vcs.php @@ -513,6 +513,7 @@ class Horde_Vcs ksort($opts); $cacheId = implode('|', array($class, $this->sourceroot(), $filename, serialize($opts), $this->_cacheVersion)); + $fetchedFromCache = false; if (!empty($this->_cache)) { // TODO: Can't use filemtime() - Git bare repos contain no files @@ -523,15 +524,15 @@ class Horde_Vcs } if ($this->_cache->exists($cacheId, $ctime)) { $ob = unserialize($this->_cache->get($cacheId, $ctime)); - $ob->setRepository($this); - return $ob; + $fetchedFromCache = true; } } - $ob = new $class($this, $filename, $opts); + if (!$ob) { $ob = new $class($filename, $opts); } + $ob->setRepository($this); $ob->applySort(self::SORT_AGE); - if (!empty($this->_cache)) { + if (!empty($this->_cache) && !$fetchedFromCache) { $this->_cache->set($cacheId, serialize($ob)); } @@ -551,12 +552,12 @@ class Horde_Vcs // Individual revisions can be cached forever if ($this->_cache->exists($cacheId, 0)) { $ob = unserialize($this->_cache->get($cacheId, 0)); - $ob->setRepository($this); - return $ob; } } - $ob = new $class($this, $fl, $rev); + if (!$ob) { $ob = new $class($rev); } + $ob->setRepository($this); + $ob->setFile($fl); if (!is_null($rev) && !empty($this->_cache)) { $this->_cache->set($cacheId, serialize($ob)); @@ -829,7 +830,6 @@ abstract class Horde_Vcs_Directory { return array(); } - } /** @@ -878,21 +878,36 @@ abstract class Horde_Vcs_File * Create a repository file object, and give it information about * what its parent directory and repository objects are. * - * @param TODO $rep TODO - * @param string $fl Full path to this file. - * @param array $opts TODO + * @param string $filename Full path to this file. + * @param array $opts TODO */ - public function __construct($rep, $fl, $opts = array()) + public function __construct($filename, $opts = array()) { - $this->_rep = $rep; - $this->_name = basename($fl); - $this->_dir = dirname($fl); + $this->_name = basename($filename); + $this->_dir = dirname($filename); + $this->_quicklog = !empty($opts['quicklog']); if (!empty($opts['branch'])) { $this->_branch = $opts['branch']; } } + protected function _ensureRevisionsInitialized() + { + } + + protected function _ensureLogsInitialized() + { + } + + /** + * When serializing, don't return the repository object + */ + public function __sleep() + { + return array_diff(array_keys(get_object_vars($this)), array('_rep')); + } + /** * TODO */ @@ -902,6 +917,14 @@ abstract class Horde_Vcs_File } /** + * TODO - better name, wrap an object around this? + */ + public function getBlob($revision) + { + return $this->_rep->checkout($this->queryPath(), $revision); + } + + /** * Has the file been deleted? * * @return boolean Is this file deleted? @@ -939,6 +962,7 @@ abstract class Horde_Vcs_File */ public function queryRevision() { + $this->_ensureRevisionsInitialized(); if (!isset($this->_revs[0])) { throw new Horde_Vcs_Exception('No revisions'); } @@ -950,6 +974,7 @@ abstract class Horde_Vcs_File */ public function queryPreviousRevision($rev) { + $this->_ensureRevisionsInitialized(); $key = array_search($rev, $this->_revs); return (($key !== false) && isset($this->_revs[$key + 1])) ? $this->_revs[$key + 1] @@ -964,6 +989,8 @@ abstract class Horde_Vcs_File */ public function queryLastLog() { + $this->_ensureRevisionsInitialized(); + $this->_ensureLogsInitialized(); if (!isset($this->_revs[0]) || !isset($this->logs[$this->_revs[0]])) { throw new Horde_Vcs_Exception('No revisions'); } @@ -979,6 +1006,8 @@ abstract class Horde_Vcs_File */ public function applySort($how = Horde_Vcs::SORT_REV) { + $this->_ensureLogsInitialized(); + switch ($how) { case Horde_Vcs::SORT_NAME: $func = 'Name'; @@ -1060,6 +1089,7 @@ abstract class Horde_Vcs_File */ public function queryLogs($rev = null) { + $this->_ensureLogsInitialized(); return is_null($rev) ? $this->logs : (isset($this->logs[$rev]) ? $this->logs[$rev] : null); @@ -1070,6 +1100,7 @@ abstract class Horde_Vcs_File */ public function revisionCount() { + $this->_ensureRevisionsInitialized(); return count($this->_revs); } @@ -1080,7 +1111,6 @@ abstract class Horde_Vcs_File { return array(); } - } /** @@ -1105,13 +1135,23 @@ abstract class Horde_Vcs_Log /** * Constructor. */ - public function __construct($rep, $fl, $rev) + public function __construct($rev) { - $this->_rep = $rep; - $this->_file = $fl; $this->_rev = $rev; } + protected function _ensureInitialized() + { + } + + /** + * When serializing, don't return the repository object + */ + public function __sleep() + { + return array_diff(array_keys(get_object_vars($this)), array('_file', '_rep')); + } + /** * TODO */ @@ -1120,20 +1160,26 @@ abstract class Horde_Vcs_Log $this->_rep = $rep; } + public function setFile(Horde_Vcs_File $file) + { + $this->_file = $file; + } + /** * TODO */ - public function queryDate() + public function queryRevision() { - return $this->_date; + return $this->_rev; } /** * TODO */ - public function queryRevision() + public function queryDate() { - return $this->_rev; + $this->_ensureInitialized(); + return $this->_date; } /** @@ -1141,6 +1187,7 @@ abstract class Horde_Vcs_Log */ public function queryAuthor() { + $this->_ensureInitialized(); return $this->_author; } @@ -1149,6 +1196,7 @@ abstract class Horde_Vcs_Log */ public function queryLog() { + $this->_ensureInitialized(); return $this->_log; } @@ -1157,6 +1205,7 @@ abstract class Horde_Vcs_Log */ public function queryBranch() { + $this->_ensureInitialized(); return array(); } @@ -1165,6 +1214,7 @@ abstract class Horde_Vcs_Log */ public function queryChangedLines() { + $this->_ensureInitialized(); return $this->_lines; } @@ -1173,6 +1223,7 @@ abstract class Horde_Vcs_Log */ public function queryTags() { + $this->_ensureInitialized(); return $this->_tags; } @@ -1185,6 +1236,8 @@ abstract class Horde_Vcs_Log */ public function querySymbolicBranches() { + $this->_ensureInitialized(); + $symBranches = array(); $branches = $this->_file->queryBranches(); @@ -1202,11 +1255,11 @@ abstract class Horde_Vcs_Log */ public function queryFiles($file = null) { + $this->_ensureInitialized(); return is_null($file) ? $this->_files : (isset($this->_files[$file]) ? $this->_files[$file] : array()); } - } /** @@ -1257,5 +1310,4 @@ abstract class Horde_Vcs_Patchset { return $this->_patchsets; } - } diff --git a/framework/Vcs/lib/Horde/Vcs/Git.php b/framework/Vcs/lib/Horde/Vcs/Git.php index 1c8d90869..ca5ed0e7f 100644 --- a/framework/Vcs/lib/Horde/Vcs/Git.php +++ b/framework/Vcs/lib/Horde/Vcs/Git.php @@ -416,19 +416,26 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File protected $_revlist = array(); /** - * Create a repository file object, and give it information about - * what its parent directory and repository objects are. + * Have we initalized logs and revisions? * - * @param TODO $rep TODO - * @param string $fl Full path to this file. - * @param array $opts TODO - * - * @throws Horde_Vcs_Exception + * @var boolean */ - public function __construct($rep, $fl, $opts = array()) + private $_initialized = false; + + protected function _ensureRevisionsInitialized() { - parent::__construct($rep, $fl, $opts); + if (!$this->_initialized) { $this->_init(); } + $this->_initialized = true; + } + protected function _ensureLogsInitialized() + { + if (!$this->_initialized) { $this->_init(); } + $this->_initialized = true; + } + + protected function _init() + { $log_list = null; /* First, grab the master list of revisions. If quicklog is specified, @@ -436,13 +443,13 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File * most recent revision for the given branch. */ if ($this->_quicklog) { $branchlist = empty($this->_branch) - ? array($rep->getDefaultBranch()) + ? array($this->_rep->getDefaultBranch()) : array($this->_branch); } else { - if (version_compare($rep->version, '1.6.0', '>=')) { - $cmd = $rep->getCommand() . ' rev-list --branches -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; + if (version_compare($this->_rep->version, '1.6.0', '>=')) { + $cmd = $this->_rep->getCommand() . ' rev-list --branches -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; } else { - $cmd = $rep->getCommand() . ' branch -v --no-abbrev'; + $cmd = $this->_rep->getCommand() . ' branch -v --no-abbrev'; exec($cmd, $branch_heads); if (stripos($branch_heads[0], 'fatal') === 0) { throw new Horde_Vcs_Exception(implode(', ', $branch_heads)); @@ -452,13 +459,13 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File $hd = $line[1]; } - $cmd = $rep->getCommand() . ' rev-list ' . implode(' ', $branch_heads) . ' -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; + $cmd = $this->_rep->getCommand() . ' rev-list ' . implode(' ', $branch_heads) . ' -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; } exec($cmd, $revs); if (count($revs) == 0) { - if (!$rep->isFile($fl, isset($opts['branch']) ? $opts['branch'] : null)) { - throw new Horde_Vcs_Exception('No such file: ' . $fl); + if (!$this->_rep->isFile($this->queryModulePath(), isset($opts['branch']) ? $opts['branch'] : null)) { + throw new Horde_Vcs_Exception('No such file: ' . $this->queryModulePath()); } else { throw new Horde_Vcs_Exception('No revisions found'); } @@ -477,7 +484,7 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File * those on $this->_branch, for branch determination reasons. */ foreach ($branchlist as $key) { $revs = array(); - $cmd = $rep->getCommand() . ' rev-list ' . ($this->_quicklog ? '-n 1' : '') . ' ' . escapeshellarg($key) . ' -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; + $cmd = $this->_rep->getCommand() . ' rev-list ' . ($this->_quicklog ? '-n 1' : '') . ' ' . escapeshellarg($key) . ' -- ' . escapeshellarg($this->queryModulePath()) . ' 2>&1'; exec($cmd, $revs); if (!empty($revs)) { @@ -504,7 +511,7 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File } foreach ($log_list as $val) { - $this->logs[$val] = $rep->getLogObject($this, $val); + $this->logs[$val] = $this->_rep->getLogObject($this, $val); } } @@ -607,7 +614,6 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File throw new Horde_Vcs_Exception('No revisions'); } - } /** @@ -625,17 +631,23 @@ class Horde_Vcs_Log_Git extends Horde_Vcs_Log protected $_parent = null; /** - * Constructor. - * - * @throws Horde_Vcs_Exception + * @var boolean */ - public function __construct($rep, $fl, $rev) + private $_initialized; + + protected function _ensureInitialized() { - parent::__construct($rep, $fl, $rev); + if (!$this->_initialized) { + $this->_init(); + $this->_initialized = true; + } + } + protected function _init() + { /* Get diff statistics. */ $stats = array(); - $cmd = $rep->getCommand() . ' diff-tree --numstat ' . escapeshellarg($rev); + $cmd = $this->_rep->getCommand() . ' diff-tree --numstat ' . escapeshellarg($this->_rev); exec($cmd, $output); reset($output); @@ -647,13 +659,12 @@ class Horde_Vcs_Log_Git extends Horde_Vcs_Log } // @TODO use Commit, CommitDate, and Merge properties - $cmd = $rep->getCommand() . ' whatchanged --no-color --pretty=format:"Rev:%H%nParents:%P%nAuthor:%an <%ae>%nAuthorDate:%at%nRefs:%d%n%n%s%n%b" --no-abbrev -n 1 ' . escapeshellarg($rev); + $cmd = $this->_rep->getCommand() . ' whatchanged --no-color --pretty=format:"Rev:%H%nParents:%P%nAuthor:%an <%ae>%nAuthorDate:%at%nRefs:%d%n%n%s%n%b" --no-abbrev -n 1 ' . escapeshellarg($this->_rev); $pipe = popen($cmd, 'r'); if (!is_resource($pipe)) { throw new Horde_Vcs_Exception('Unable to run ' . $cmd . ': ' . error_get_last()); } - //$line = trim(fgets($pipe)); while (true) { $line = trim(fgets($pipe)); if (!strlen($line)) { break; } @@ -666,9 +677,9 @@ class Horde_Vcs_Log_Git extends Horde_Vcs_Log switch (trim($key)) { case 'Rev': - if ($rev != $value) { + if ($this->_rev != $value) { fclose($pipe); - throw new Horde_Vcs_Exception('Expected ' . $rev . ', got ' . $value); + throw new Horde_Vcs_Exception('Expected ' . $this->_rev . ', got ' . $value); } break; diff --git a/framework/Vcs/lib/Horde/Vcs/Svn.php b/framework/Vcs/lib/Horde/Vcs/Svn.php index 24266046f..dc07df335 100644 --- a/framework/Vcs/lib/Horde/Vcs/Svn.php +++ b/framework/Vcs/lib/Horde/Vcs/Svn.php @@ -124,7 +124,7 @@ class Horde_Vcs_Svn extends Horde_Vcs */ public function checkout($fullname, $rev) { - $this->assertValidRevision($rev); + $this->_rep->assertValidRevision($rev); if ($RCS = popen($this->getCommand() . ' cat -r ' . escapeshellarg($rev) . ' ' . escapeshellarg($fullname) . ' 2>&1', VC_WINDOWS ? 'rb' : 'r')) { return $RCS; @@ -282,23 +282,32 @@ class Horde_Vcs_File_Svn extends Horde_Vcs_File public $logpipe; /** - * Create a repository file object, and give it information about - * what its parent directory and repository objects are. + * Have we initalized logs and revisions? * - * @param TODO $rep TODO - * @param string $fl Full path to this file. - * @param array $opts TODO + * @var boolean */ - public function __construct($rep, $fl, $opts = array()) + private $_initialized = false; + + protected function _ensureRevisionsInitialized() + { + if (!$this->_initialized) { $this->_init(); } + $this->_initialized = true; + } + + protected function _ensureLogsInitialized() { - parent::__construct($rep, $fl, $opts); - - /* This doesn't work; need to find another way to simply - * request the most recent revision: - * - * $flag = $this->_quicklog ? '-r HEAD ' : ''; - */ - $cmd = $rep->getCommand() . ' log -v ' . escapeshellarg($this->queryFullPath()) . ' 2>&1'; + if (!$this->_initialized) { $this->_init(); } + $this->_initialized = true; + } + + protected function _init() + { + // This doesn't work; need to find another way to simply + // request the most recent revision: + // + // $flag = $this->_quicklog ? '-r HEAD ' : ''; + + $cmd = $this->_rep->getCommand() . ' log -v ' . escapeshellarg($this->queryFullPath()) . ' 2>&1'; $pipe = popen($cmd, 'r'); if (!$pipe) { throw new Horde_Vcs_Exception('Failed to execute svn log: ' . $cmd); @@ -310,10 +319,9 @@ class Horde_Vcs_File_Svn extends Horde_Vcs_File } $this->logpipe = $pipe; - while (!feof($pipe)) { try { - $log = $rep->getLogObject($this, null); + $log = $this->_rep->getLogObject($this, null); $rev = $log->queryRevision(); $this->logs[$rev] = $log; $this->_revs[] = $rev; @@ -354,15 +362,25 @@ class Horde_Vcs_Log_Svn extends Horde_Vcs_Log protected $_files = array(); /** - * Constructor. + * @var boolean */ - public function __construct($rep, $fl, $rev) - { - parent::__construct($rep, $fl, $rev); + private $_initialized; - $line = fgets($fl->logpipe); + protected function _ensureInitialized() + { + if (!$this->_initialized) { + $this->_init(); + $this->_initialized = true; + } + } - if (feof($fl->logpipe) || !$line) { + /** + * Constructor. + */ + protected function _init() + { + $line = fgets($this->_file->logpipe); + if (feof($this->_file->logpipe) || !$line) { throw new Horde_Vcs_Exception('No more data'); } @@ -375,18 +393,18 @@ class Horde_Vcs_Log_Svn extends Horde_Vcs_Log throw new Horde_Vcs_Exception('SVN Error'); } - fgets($fl->logpipe); + fgets($this->_file->logpipe); - while (($line = trim(fgets($fl->logpipe))) != '') { + while (($line = trim(fgets($this->_file->logpipe))) != '') { $this->_files[] = $line; } for ($i = 0; $i != $size; ++$i) { - $this->_log = $this->_log . chop(fgets($fl->logpipe)) . "\n"; + $this->_log = $this->_log . chop(fgets($this->_file->logpipe)) . "\n"; } $this->_log = chop($this->_log); - fgets($fl->logpipe); + fgets($this->_file->logpipe); } /** -- 2.11.0