- refs #374 Performance: Fix memory leak during Archiving process

refactoring and fixing architecture issue, which makes code more simple
matomo-org · Mar 10, 2009 · 63022a1 · 63022a1
1 parent 68917a0
commit 63022a1
Show file tree

Hide file tree

Showing 16 changed files with 83 additions and 197 deletions.
diff --git a/core/ArchiveProcessing.php b/core/ArchiveProcessing.php
@@ -11,6 +11,8 @@
 
 require_once 'TablePartitioning.php';
 require_once 'ArchiveProcessing/Record.php';
+require_once 'ArchiveProcessing/RecordArray.php';
+require_once "ArchiveProcessing/Record/Numeric.php";
 require_once 'DataTable.php';
 
 /**
@@ -325,11 +327,7 @@ abstract protected function compute();
 	protected function initCompute()
 	{
 		$this->loadNextIdarchive();
-
-		$record = new Piwik_ArchiveProcessing_Record_Numeric('done', Piwik_ArchiveProcessing::DONE_ERROR);
-		$this->insertRecord($record);
-		$record->delete();
-
+		$this->insertNumericRecord('done', Piwik_ArchiveProcessing::DONE_ERROR);
 		$this->logTable 			= Piwik::prefixTable('log_visit');
 		$this->logVisitActionTable 	= Piwik::prefixTable('log_link_visit_action');
 		$this->logActionTable	 	= Piwik::prefixTable('log_action');
@@ -353,25 +351,8 @@ protected function postCompute()
 					array($this->idArchive)
 				);
 
-		$record = new Piwik_ArchiveProcessing_Record_Numeric('done', Piwik_ArchiveProcessing::DONE_OK);
-
-		// save in the database the records
-		$records = Piwik_ArchiveProcessing_Record_Manager::getInstance()->getRecords();
+		$this->insertNumericRecord('done', Piwik_ArchiveProcessing::DONE_OK);
 
-		foreach($records as $record)
-		{
-			$this->insertRecord( $record);	
-		}
-
-		// delete the records from the global manager
-		foreach($records as $record)
-		{
-			$record->delete();	
-		}
-		unset($records);
-
-		// we delete all tables from the table register
-		Piwik_ArchiveProcessing_Record_Manager::getInstance()->deleteAll();
 		Piwik_DataTable_Manager::getInstance()->deleteAll();
 	}
 
@@ -468,13 +449,46 @@ protected function loadNextIdarchive()
 		$this->idArchive = $id + 1;
 
 	}
+
+	/**
+	 * @param string $name
+	 * @param int|float $value
+	 * @return void
+	 */
+	public function insertNumericRecord($name, $value)
+	{
+		$record = new Piwik_ArchiveProcessing_Record_Numeric($name, $value);
+		$this->insertRecord($record);
+	}
+
+	/**
+	 * @param string $name
+	 * @param string|array of string $aValues
+	 * @return void
+	 */
+	public function insertBlobRecord($name, $value)
+	{
+		if(is_array($value))
+		{
+			$records = new Piwik_ArchiveProcessing_RecordArray($name, $value);
+			foreach($records->get() as $record)
+			{
+				$this->insertRecord($record);
+			}
+		}
+		else
+		{
+			$record = new Piwik_ArchiveProcessing_Record_Blob($name, $value);
+			$this->insertRecord($record);
+		}
+	}
 
 	/**
 	 * Inserts a record in the right table (either NUMERIC or BLOB)
 	 *
 	 * @param Piwik_ArchiveProcessing_Record $record
 	 */
-	protected function insertRecord($record)
+	private function insertRecord($record)
 	{
 		// table to use to save the data
 		if(Piwik::isNumeric($record->value))

diff --git a/core/ArchiveProcessing/Day.php b/core/ArchiveProcessing/Day.php
@@ -70,7 +70,7 @@ protected function compute()
 
 		foreach($row as $name => $value)
 		{
-			$record = new Piwik_ArchiveProcessing_Record_Numeric($name, $value);
+			$this->insertNumericRecord($name, $value);
 		}
 		$this->setNumberOfVisits($row['nb_visits']);
 		$this->setNumberOfVisitsConverted($row['nb_visits_converted']);

diff --git a/core/ArchiveProcessing/Period.php b/core/ArchiveProcessing/Period.php
@@ -124,7 +124,7 @@ private function archiveNumericValuesGeneral($aNames, $operationToApply)
 	 * It will usually be called in a plugin that listens to the hook 'ArchiveProcessing_Period.compute'
 	 * 
 	 * For example if $aRecordName = 'UserCountry_country' the method will select all UserCountry_country DataTable for the period
-	 * (eg. the 31 dataTable of the last month), sum them, and create the Piwik_ArchiveProcessing_Record_BlobArray so that
+	 * (eg. the 31 dataTable of the last month), sum them, and create the Piwik_ArchiveProcessing_RecordArray so that
 	 * the resulting dataTable is AUTOMATICALLY recorded in the database.
 	 * 
 	 * 
@@ -155,7 +155,7 @@ public function archiveDataTable( $aRecordName, $maximumRowsInDataTableLevelZero
 			$nameToCount[$recordName]['level0'] =  $table->getRowsCount();
 			$nameToCount[$recordName]['recursive'] =  $table->getRowsCountRecursive();
 
-			$record = new Piwik_ArchiveProcessing_Record_BlobArray($recordName, $table->getSerialized( $maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable ));
+			$record = new Piwik_ArchiveProcessing_RecordArray($recordName, $table->getSerialized( $maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable ));
 		}
 		return $nameToCount;
 	}

diff --git a/core/ArchiveProcessing/Record.php b/core/ArchiveProcessing/Record.php
@@ -9,12 +9,6 @@
  * @package Piwik_ArchiveProcessing
  */
 
-require_once "ArchiveProcessing/Record/Blob.php";
-require_once "ArchiveProcessing/Record/BlobArray.php";
-require_once "ArchiveProcessing/Record/Numeric.php";
-require_once "ArchiveProcessing/Record/Manager.php";
-
-
 /**
  * A Record is a tuple (name, value) to be saved in the database.
  * At its creation, the record registers itself to the RecordManager. 
@@ -38,12 +32,6 @@ function __construct( $name, $value)
 	{
 		$this->name = $name;
 		$this->value = $value;
-		Piwik_ArchiveProcessing_Record_Manager::getInstance()->registerRecord($this);
-	}
-
-	public function delete()
-	{
-		Piwik_ArchiveProcessing_Record_Manager::getInstance()->unregister($this);
 	}
 
 	public function __destruct()

diff --git a/core/ArchiveProcessing/Record/Blob.php b/core/ArchiveProcessing/Record/Blob.php
@@ -21,11 +21,13 @@ class Piwik_ArchiveProcessing_Record_Blob extends Piwik_ArchiveProcessing_Record
 {
 	public $name;
 	public $value;
+
 	function __construct( $name, $value)
 	{
 		$value = gzcompress($value);
 		parent::__construct( $name, $value );
 	}
+
 	public function __toString()
 	{
 		return $this->name ." = BLOB";//". gzuncompress($this->value);

diff --git a/core/ArchiveProcessing/Record/Manager.php b/core/ArchiveProcessing/Record/Manager.php
diff --git a/core/ArchiveProcessing/Record/BlobArray.php → core/ArchiveProcessing/RecordArray.php b/core/ArchiveProcessing/Record/BlobArray.php → core/ArchiveProcessing/RecordArray.php
@@ -9,11 +9,12 @@
  * @package Piwik_ArchiveProcessing
  */
 
+require_once "ArchiveProcessing/Record/Blob.php";
 /**
  * Array of blob records.
  * Useful for easily saving splited data in the DB.
  *  
- * Example: $record = new Piwik_ArchiveProcessing_Record_BlobArray(
+ * Example: $record = new Piwik_ArchiveProcessing_RecordArray(
  * 				'veryLongBook', 
  * 				0 => serialize(	array( '1st chapter very long, 6MB of data we dont want to save' )),
  * 				1 => serialize(	array( '2nd chapter very long, 8MB of data we dont want to save' )),
@@ -30,8 +31,10 @@
  * @package Piwik_ArchiveProcessing
  * @subpackage Piwik_ArchiveProcessing_Record
  */
-class Piwik_ArchiveProcessing_Record_BlobArray extends Piwik_ArchiveProcessing_Record
+class Piwik_ArchiveProcessing_RecordArray extends Piwik_ArchiveProcessing_Record
 {
+	protected $records = array();
+
 	function __construct( $name, $aValue)
 	{		
 		foreach($aValue as $id => $value)
@@ -47,17 +50,12 @@ function __construct( $name, $aValue)
 			{
 				$newName = $name . '_' . $id;
 			}
-			$record = new Piwik_ArchiveProcessing_Record_Blob( $newName,  $value );
+			$this->records[] = new Piwik_ArchiveProcessing_Record_Blob( $newName,  $value );
 		}
 	}
 
-	public function __toString()
-	{
-		throw new Exception( 'Not valid' );
-	}
-
-	public function delete()
+	public function get()
 	{
-		throw new Exception( 'Not valid' );
+		return $this->records;
 	}
 }
diff --git a/plugins/Actions/Actions.php b/plugins/Actions/Actions.php
@@ -194,15 +194,15 @@ protected function archiveDayRecordInDatabase()
 
 		$dataTable = Piwik_ArchiveProcessing_Day::generateDataTable($this->actionsTablesByType[Piwik_Tracker_Action::TYPE_ACTION]);
 		$s = $dataTable->getSerialized( $maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable );
-		$record = new Piwik_ArchiveProcessing_Record_BlobArray('Actions_actions', $s);
+		$archiveProcessing->insertBlobRecord('Actions_actions', $s);
 
 		$dataTable = Piwik_ArchiveProcessing_Day::generateDataTable($this->actionsTablesByType[Piwik_Tracker_Action::TYPE_DOWNLOAD]);
 		$s = $dataTable->getSerialized($maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable );
-		$record = new Piwik_ArchiveProcessing_Record_BlobArray('Actions_downloads', $s);
+		$archiveProcessing->insertBlobRecord('Actions_downloads', $s);
 
 		$dataTable = Piwik_ArchiveProcessing_Day::generateDataTable($this->actionsTablesByType[Piwik_Tracker_Action::TYPE_OUTLINK]);
 		$s = $dataTable->getSerialized( $maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable );
-		$record = new Piwik_ArchiveProcessing_Record_BlobArray('Actions_outlink', $s);
+		$archiveProcessing->insertBlobRecord('Actions_outlink', $s);
 
 		unset($this->actionsTablesByType);
 	}

diff --git a/plugins/Goals/Goals.php b/plugins/Goals/Goals.php
@@ -129,14 +129,13 @@ function archivePeriod($notification )
 		{
 			$nb_conversions = $records[self::getRecordName('nb_conversions', $goalId)]->value;
 			$conversion_rate = $this->getConversionRate($nb_conversions, $archiveProcessing);
-			$record = new Piwik_ArchiveProcessing_Record_Numeric(self::getRecordName('conversion_rate', $goalId), $conversion_rate);
+			$archiveProcessing->insertNumericRecord(self::getRecordName('conversion_rate', $goalId), $conversion_rate);
 		}
 
 		// global conversion rate
 		$nb_conversions = $records[self::getRecordName('nb_conversions')]->value;
 		$conversion_rate = $this->getConversionRate($nb_conversions, $archiveProcessing);
-		$record = new Piwik_ArchiveProcessing_Record_Numeric(self::getRecordName('conversion_rate'), $conversion_rate);
-
+		$archiveProcessing->insertNumericRecord(self::getRecordName('conversion_rate'), $conversion_rate);
 	}
 
 	function archiveDay( $notification )
@@ -169,11 +168,11 @@ function archiveDay( $notification )
 			{
 				$metricName = Piwik_Archive::$mappingFromIdToNameGoal[$metricId];
 				$recordName = self::getRecordName($metricName, $idgoal);
-				$record = new Piwik_ArchiveProcessing_Record_Numeric($recordName, $value);
+				$archiveProcessing->insertNumericRecord($recordName, $value);
 			}
 			$conversion_rate = $this->getConversionRate($values[Piwik_Archive::INDEX_GOAL_NB_CONVERSIONS], $archiveProcessing);
 			$recordName = self::getRecordName('conversion_rate', $idgoal);
-			$record = new Piwik_ArchiveProcessing_Record_Numeric($recordName, $conversion_rate);
+			$archiveProcessing->insertNumericRecord($recordName, $conversion_rate);
 		}
 
 		// Stats by goal, for visitor returning / non returning
@@ -185,7 +184,7 @@ function archiveDay( $notification )
 				{
 					$metricName = Piwik_Archive::$mappingFromIdToNameGoal[$metricId];
 					$recordName = self::getRecordName($metricName, $idgoal, $visitor_returning);
-					$record = new Piwik_ArchiveProcessing_Record_Numeric($recordName, $value);
+					$archiveProcessing->insertNumericRecord($recordName, $value);
 //					echo $record . "<br>";
 				}
 			}
@@ -199,7 +198,7 @@ function archiveDay( $notification )
 		);
 		foreach($totalAllGoals as $recordName => $value)
 		{
-			$record = new Piwik_ArchiveProcessing_Record_Numeric($recordName, $value);
+			$archiveProcessing->insertNumericRecord($recordName, $value);
 		}
 	}
 

diff --git a/plugins/Provider/Provider.php b/plugins/Provider/Provider.php
@@ -99,7 +99,7 @@ function archiveDay($notification)
 		$labelSQL = "location_provider";
 		$interestByProvider = $archiveProcessing->getArrayInterestForLabel($labelSQL);
 		$tableProvider = $archiveProcessing->getDataTableFromArray($interestByProvider);
-		$record = new Piwik_ArchiveProcessing_Record_BlobArray($recordName, $tableProvider->getSerialized());
+		$archiveProcessing->insertBlobRecord($recordName, $tableProvider->getSerialized());
 	}
 
 	/**