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.

diff --git a/framework/Vcs/lib/Horde/Vcs.php b/framework/Vcs/lib/Horde/Vcs.php
index c29a79b..ed93d2b 100644 (file)
--- 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 1c8d908..ca5ed0e 100644 (file)
--- 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 2426604..dc07df3 100644 (file)
--- 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);
     }
 
     /**