DDC-217 - Refactor result cache again to be much simpler, previous approach...

DDC-217 - Refactor result cache again to be much simpler, previous approach was too complicated and something like this needs longer testing. Now only caching the full result.

lib/Doctrine/DBAL/Cache/RowCacheStatement.php → lib/Doctrine/DBAL/Cache/ResultCacheStatement.php

@@ -36,12 +36,12 @@ use Doctrine\DBAL\Connection;
  * Also you have to realize that the cache will load the whole result into memory at once to ensure 2.
  * This means that the memory usage for cached results might increase by using this feature.
  */
-class RowCacheStatement implements ResultStatement
+class ResultCacheStatement implements ResultStatement
 {
     /**
      * @var \Doctrine\Common\Cache\Cache
      */
-    private $cache;
+    private $resultCache;
 
     /**
      *
@@ -49,6 +49,11 @@ class RowCacheStatement implements ResultStatement
      */
     private $cacheKey;
 
+    /**
+     * @var string
+     */
+    private $realKey;
+
     /**
      * @var int
      */
@@ -59,16 +64,6 @@ class RowCacheStatement implements ResultStatement
      */
     private $statement;
 
-    /**
-     * @var array
-     */
-    private $rowPointers = array();
-
-    /**
-     * @var int
-     */
-    private $num = 0;
-
     /**
      * Did we reach the end of the statement?
      * 
@@ -85,25 +80,27 @@ class RowCacheStatement implements ResultStatement
      * @param array $types
      * @return RowCacheStatement
      */
-    static public function create(Connection $conn, $cacheKey, $lifetime, $query, $params, $types)
+    static public function create(Connection $conn, $query, $params, $types, $lifetime = 0, $cacheKey = null)
     {
         $resultCache = $conn->getConfiguration()->getResultCacheImpl();
         if (!$resultCache) {
             return $conn->executeQuery($query, $params, $types);
         }
 
-        if ($rowPointers = $resultCache->fetch($cacheKey)) {
-            $data = array();
-            foreach ($rowPointers AS $rowPointer) {
-                if ($row = $resultCache->fetch($rowPointer)) {
-                    $data[] = $row;
-                } else {
-                    return new self($conn->executeQuery($query, $params, $types), $resultCache, $cacheKey, $lifetime);
-                }
+        $realKey = $query . "-" . serialize($params) . "-" . serialize($types);
+        // should the key be automatically generated using the inputs or is the cache key set?
+        if ($cacheKey === null) {
+            $cacheKey = sha1($realKey);
+        }
+
+        // fetch the row pointers entry
+        if ($data = $resultCache->fetch($cacheKey)) {
+            // is the real key part of this row pointers map or is the cache only pointing to other cache keys?
+            if (isset($data[$realKey])) {
+                return new ArrayStatement($data[$realKey]);
             }
-            return new ArrayStatement($data);
         }
-        return new self($conn->executeQuery($query, $params, $types), $resultCache, $cacheKey, $lifetime);
+        return new self($conn->executeQuery($query, $params, $types), $resultCache, $cacheKey, $realKey, $lifetime);
     }
 
     /**
@@ -113,11 +110,12 @@ class RowCacheStatement implements ResultStatement
      * @param string $cacheKey
      * @param int $lifetime
      */
-    public function __construct($stmt, $resultCache, $cacheKey, $lifetime = 0)
+    private function __construct($stmt, $resultCache, $cacheKey, $realKey, $lifetime)
     {
         $this->statement = $stmt;
         $this->resultCache = $resultCache;
         $this->cacheKey = $cacheKey;
+        $this->realKey = $realKey;
         $this->lifetime = $lifetime;
     }
 
@@ -128,11 +126,15 @@ class RowCacheStatement implements ResultStatement
      */
     public function closeCursor()
     {
-        // the "important" key is written as the last one. This way we ensure it has a longer lifetime than the rest
-        // avoiding potential cache "misses" during the reconstruction.
-        if ($this->emptied && $this->rowPointers) {
-            $this->resultCache->save($this->cacheKey, $this->rowPointers, $this->lifetime);
-            unset($this->rowPointers);
+        if ($this->emptied && $this->data) {
+            $data = $this->resultCache->fetch($this->cacheKey);
+            if (!$data) {
+                $data = array();
+            }
+            $data[$this->realKey] = $this->data;
+
+            $this->resultCache->save($this->cacheKey, $data, $this->lifetime);
+            unset($this->data);
         }
     }
 
@@ -180,9 +182,8 @@ class RowCacheStatement implements ResultStatement
     {
         $row = $this->statement->fetch(PDO::FETCH_ASSOC);
         if ($row) {
-            $rowCacheKey = $this->cacheKey . "#row". ($this->num++);
-            $this->rowPointers[] = $rowCacheKey;
-            $this->resultCache->save($rowCacheKey, $row, $this->lifetime);
+            $this->data[] = $row;
+            
             if ($fetchStyle == PDO::FETCH_ASSOC) {
                 return $row;
             } else if ($fetchStyle == PDO::FETCH_NUM) {

lib/Doctrine/DBAL/Connection.php

@@ -24,7 +24,7 @@ use PDO, Closure, Exception,
     Doctrine\DBAL\Driver\Connection as DriverConnection,
     Doctrine\Common\EventManager,
     Doctrine\DBAL\DBALException,
-    Doctrine\DBAL\Cache\RowCacheStatement;
+    Doctrine\DBAL\Cache\ResultCacheStatement;
 
 /**
  * A wrapper around a Doctrine\DBAL\Driver\Connection that adds features like
@@ -595,15 +595,16 @@ class Connection implements DriverConnection
      * @param string $query The SQL query to execute.
      * @param array $params The parameters to bind to the query, if any.
      * @param array $types The types the previous parameters are in.
+     * @param int $useCacheLifetime lifetime of the cache result, set to true for infinite lifetime.
      * @param string|null $cacheResultKey name of the result cache key.
-     * @param int $cacheLifetime lifetime of the cache result.
      * @return Doctrine\DBAL\Driver\Statement The executed statement.
      * @internal PERF: Directly prepares a driver statement, not a wrapper.
      */
-    public function executeQuery($query, array $params = array(), $types = array(), $cacheResultKey = null, $cacheLifetime = 0)
+    public function executeQuery($query, array $params = array(), $types = array(), $useCacheLifetime = false, $cacheResultKey = null)
     {
-        if ($cacheResultKey !== null) {
-            return RowCacheStatement::create($this, $cacheResultKey, $cacheLifetime, $query, $params, $types);
+        if ($useCacheLifetime !== false) {
+            $useCacheLifetime = $useCacheLifetime === true ? 0 : $useCacheLifetime;
+            return ResultCacheStatement::create($this, $query, $params, $types, $useCacheLifetime, $cacheResultKey);
         }
 
         $this->connect();

lib/Doctrine/DBAL/Portability/Connection.php

@@ -86,9 +86,9 @@ class Connection extends \Doctrine\DBAL\Connection
         return $this->case;
     }
     
-    public function executeQuery($query, array $params = array(), $types = array())
+    public function executeQuery($query, array $params = array(), $types = array(), $useCacheLifetime = false, $cacheResultKey = null)
     {
-        return new Statement(parent::executeQuery($query, $params, $types), $this);
+        return new Statement(parent::executeQuery($query, $params, $types, $useCacheLifetime, $cacheResultKey), $this);
     }
     
     /**

tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php

@@ -36,7 +36,9 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
 
         $config = $this->_conn->getConfiguration();
         $config->setSQLLogger($this->sqlLogger = new \Doctrine\DBAL\Logging\DebugStack);
-        $config->setResultCacheImpl(new \Doctrine\Common\Cache\ArrayCache);
+
+        $cache = new \Doctrine\Common\Cache\ArrayCache;
+        $config->setResultCacheImpl($cache);
     }
 
     public function testCacheFetchAssoc()
@@ -68,7 +70,7 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
         foreach ($this->expectedResult AS $v) {
             $numExpectedResult[] = array_values($v);
         }
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $data = array();
         while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
@@ -78,7 +80,7 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
 
         $this->assertEquals($this->expectedResult, $data);
 
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $data = array();
         while ($row = $stmt->fetch(\PDO::FETCH_NUM)) {
@@ -91,14 +93,14 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
 
     public function testDontCloseNoCache()
     {
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $data = array();
         while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
             $data[] = $row;
         }
 
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $data = array();
         while ($row = $stmt->fetch(\PDO::FETCH_NUM)) {
@@ -110,12 +112,12 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
 
     public function testDontFinishNoCache()
     {
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $row = $stmt->fetch(\PDO::FETCH_ASSOC);
         $stmt->closeCursor();
 
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $data = array();
         while ($row = $stmt->fetch(\PDO::FETCH_NUM)) {
@@ -128,7 +130,8 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
 
     public function assertCacheNonCacheSelectSameFetchModeAreEqual($expectedResult, $fetchStyle)
     {
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $s = microtime(true);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $this->assertEquals(2, $stmt->columnCount());
 
@@ -137,10 +140,12 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
             $data[] = $row;
         }
         $stmt->closeCursor();
+        #echo number_format(microtime(true)-$s, 6)."\n";
 
         $this->assertEquals($expectedResult, $data);
 
-        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), "testcachekey", 10);
+        $s = microtime(true);
+        $stmt = $this->_conn->executeQuery("SELECT * FROM caching", array(), array(), 10, "testcachekey");
 
         $this->assertEquals(2, $stmt->columnCount());
 
@@ -149,6 +154,7 @@ class ResultCacheTest extends \Doctrine\Tests\DbalFunctionalTestCase
             $data[] = $row;
         }
         $stmt->closeCursor();
+        #echo number_format(microtime(true)-$s, 6)."\n";
 
         $this->assertEquals($expectedResult, $data);