Memory leak with relational Active Record and PHP<5.3

Archives Bug Discussions
1

Hello everyone !

I am using Yii 1.1.2 and PHP 5.2.9, and I ran into memory leak problems with relational Active Record.

I searched the Yii forums for "memory leak" and found several other people have similar problems, and several good advices (disabling logging, disabling YII_DEBUG, and removing events/behaviors before deleting the records).

Unfortunately, my problem persisted even when using all these tricks, so I opened a new topic.

How to reproduce :

You need :

  • a CActiveRecord model class with at least one relation

  • PHP < 5.3 (as I guess the 5.3 and latter versions will solve the problem with the new garbage collector)

  • an Action like this one :




    public function actionTest() {         

        for($i = 0; $i<1000; $i++)  {

            modelClass::model()->with('relation')->findByPk(1);


            if(0 == $i%100) {

                var_dump(memory_get_peak_usage());

            } 

        }

    }

Run the action, and you will see the memory usage climbing.

Note you can also lazy load the relation, and this problem will still appears.

Now, I looked into the Yii source code to try and find the problem.

It seems there are at least two circular references in the CActiveFinder class, which is used to fetch the relations in the relational Active Record Process :

1- The CActiveFinder class references a CJoinElement as its _joinTree property, and this CJoinElement references the CActiveFinder as its model property.

2- The joinTree is composed of several CJoinElement which inter-reference themselves (children and parent properties).

I wrote a little hack resolving the problem :

In CActiveFinder.php :

Replace CactiveFinder::query() by




	public function query($criteria,$all=false)

	{

		$this->_joinTree->model->applyScopes($criteria);

		$this->_joinTree->beforeFind();


		$alias=$criteria->alias===null ? 't' : $criteria->alias;

		$this->_joinTree->tableAlias=$alias;

		$this->_joinTree->rawTableAlias=$this->_builder->getSchema()->quoteTableName($alias);


		$this->_joinTree->find($criteria);

		$this->_joinTree->afterFind();

        

		if($all)

			$result = array_values($this->_joinTree->records);

		else if(count($this->_joinTree->records))

                        $result = reset($this->_joinTree->records);

		else

			$result = null;


                $this->_joinTree = null; //  <---- resolves the 1st circular reference

                return $result;

	}

And CJoinElement::afterFind() by :




        public function afterFind()

	{

		foreach($this->records as $record)

			$record->afterFindInternal();

		foreach($this->children as $child)

			$child->afterFind();


                $this->children = null; //  <---- resolves the 2nd circular reference

	}

With these changes, the memory leak disappears :)

The downside of this solution is that it becomes impossible to perform several find() with a single CActiveFinder instance (but I don’t know if it may be usefull or not, and if it was even possible ^^).

If you find a better/cleaner solution, or foresee problems that may arise by using this “fix”, I would be really interested in hearing your thoughts :)

Hope this can help someone :)

2

Nice one. It solves the issue. Checking if it breaks something. If not, it will be in the next release, I think.

http://code.google.com/p/yii/source/detail?r=2196

3

When you are using some complex scopes (with calls to DB or to cache), it’s not good to can’t perform severl finds with them…

That was one of the best things of AR

4

You can do several finds with scopes:




$m = User::model()->scope1();

$users = $m->findAll();

$users = $m->scope2()->findAll();

What you can’t do is:




$userFinder = User::model()->with('posts');

$users1 = $userFinder->findAll();

$users2 = $userFinder->findAll();
5

Yes, I know…

Some times you have to compare items and only change the findByPk($iditem)

So you will have to change the code from this:




$baseAF = MyAR::model->scope1()->scope2()->with($complexWithArray);


$compareItem1 = $baseAf->findByPk($iditem1);

$compareItem2 = $baseAf->findByPk($iditem2);

But now that i’m writing i thought and i should do it like this: (lol)




$compareItems = $baseAf->fidnAllByPks($array(iditems));

But still think it will change something that maybe people is doing… So we need to write it on the guide or upgrade file…

6

I see. OK, will do it.