6

So I found out that in theory, Mage::objects() could be used to cache objects via Varien_Object_Cache.

I noticed that if I run the following:

// Test #1
for ($i = 0; $i < 100000; $i++) {
    $product = Mage::getModel('catalog/product');
}

// Test #2
$productModel = Mage::getModel('catalog/product');
$cache = Mage::objects()->save($productModel);

for ($i = 0; $i < 100000; $i++) {
    $product = Mage::objects()->load($cache);
}

The performance (in both time and memory) is crazy : the first loop takes almost 20 times more time than the second one.

So here's my concern, let's say I have a loop that imports a lot of products like this:

foreach($datas as $data) {
    $product = Mage::getModel('catalog/product');
    $product->addData($data);
    $product->save();
}

Now what if I've added the following to my class:

protected $_productModel;

public function getProductModel()
{
    if (is_null($this->_productModel)) {
        $productModel = Mage::getModel('catalog/product');
        $this->_productModel = Mage::objects()->save($productModel);
    }
    return Mage::objects()->load($this->_productModel);
}

In my loop, let's say I replace:

$product = Mage::getModel('catalog/product');

With:

$product = $this->getProductModel();

So The idea I'm referring to would be to allocate the product class to the object pool from Varien_Object_Cache (accessible via Mage::objects()) so the object from getModel doesn't have to be allocated and destroyed over and over again.

  • What risks does that imply ?
  • Is that the proper way of using Mage::objects() ?
  • What are the limits of Varien_Object_Cache, like can I cache more models such as Mage::getModel('catalog/category') ?
8
  • I haven't looked closely at the code, but it looks like this is a likely apples->oranges comparison. If I have followed the logic correctly your test should use Mage::getSingleton() not Mage::getModel(). Commented Sep 29, 2016 at 15:08
  • @KevinSchroeder my test is not entirely accurate as I don't set different data to the product. Think about this loop like it would create one different product on each iteration. Commented Sep 29, 2016 at 15:09
  • Right, and if you are using an object cache then you are re-using the object created the first time, similar to getSingleton. It is basically a registry; something best avoided unless you have a specific need that it can only fill. But it's tough to say without seeing the entire use case. Commented Sep 29, 2016 at 15:16
  • @KevinSchroeder well I'm not getting this out of nowhere actually: github.com/OpenMage/magento-mirror/blob/1.9.2.4/app/code/core/… Commented Sep 29, 2016 at 15:18
  • 1
    Never said you did. Incidentally this code here implies the same thing I said earlier about singletons: github.com/OpenMage/magento-mirror/blob/1.9.2.4/app/code/core/…. But I stand by my statement that registries should be avoided, even if the Magento core does it in many places. But, again, it's tough to say exactly without knowing the full use case. Commented Sep 29, 2016 at 15:24

1 Answer 1

5

I have to admit, this is the first time I saw Mage::objects() and Varien_Object_Cache, but after looking at the code I think it was good like that. This should stay in the dark. There is too much global state already in Magento 1.

First, I agree with Kevin Schroeder that you are comparing apples with oranges. The Varien_Object_Cache is a special form of registry, it saves instantiated objects and returns them. It does not save or return a copy (which you seem to imply)

The difference to Mage::getSingleton() is obviously that you can save many objects of the same class and the difference to Mage::registry() is that you don't have to choose a key, a new key will automatically be assigned.

There are some interesting features though:

  • If you save the same object twice, the same cache entry will be reused, so it's only once in the cache (identity is determined by spl_object_hash, and this is where it gets buggy. I'll come to that later)
  • You can add cache tags when saving an object and find or delete objects by tag
  • You can also find or delete objects by class

About spl_object_hash()

I always get suspicious if people try to be clever with this function. It has one pitfall: it is based on some internal ID and as soon as objects are garbage collected, new objects will get the same ID. So if comparing two spl_object_hash values only makes sense if both objects are in memory at the same time. This seems not to be guaranteed here, at least I don't see that entries are always removed from the array of hashes when the corresponding object is removed from the cache.

With this code

Mage::objects()->save($obj);
Mage::objects()->delete($obj);

the object will be removed from the cache but the "hash" is still there. Here's a var_dump after I added three objects and removed two:

class Varien_Object_Cache#827 (9) {
  protected $_idx =>
  int(3)
  protected $_objects =>
  array(1) {
    '#3' =>
    class stdClass#883 (1) {
      public $baz =>
      string(3) "baz"
    }
  }
  protected $_hashes =>
  array(3) {
    '00000000403b4ef2000000002079cc52' =>
    string(2) "#1"
    '00000000403b4eff000000002079cc52' =>
    string(2) "#2"
    '00000000403b4eb9000000002079cc52' =>
    string(2) "#3"
  }
  protected $_objectHashes =>
  array(3) {
    '#1' =>
    string(32) "00000000403b4ef2000000002079cc52"
    '#2' =>
    string(32) "00000000403b4eff000000002079cc52"
    '#3' =>
    string(32) "00000000403b4eb9000000002079cc52"
  }
  protected $_tags =>
  array(0) {
  }
  protected $_objectTags =>
  array(0) {
  }
  protected $_references =>
  array(0) {
  }
  protected $_objectReferences =>
  array(0) {
  }
  protected $_debug =>
  array(0) {
  }
}

If you look closely, you should also see why I used quotes around "hash". The values only have quite small differences.

The documentation for spl_object_hash() explicitly states:

When an object is destroyed, its hash may be reused for other objects.

The top user comments are also relevant:

Note that the contents (properties) of the object are NOT hashed by the function, merely its internal handle and handler table pointer. This is sufficient to guarantee that any two objects simultaneously co-residing in memory will have different hashes. Uniqueness is not guaranteed between objects that did not reside in memory simultaneously, for example:

var_dump(spl_object_hash(new stdClass()), spl_object_hash(new  stdClass()));

Running this alone will usually generate the same hashes, since PHP reuses the internal handle for the first stdClass after it has been dereferenced and destroyed when it creates the second stdClass.

and

Note that given two different objects spl_object_hash() can return values that look very similar, and in fact both the most significant and least significant digits are likely to be identical! e.g. "000000003cc56d770000000007fa48c5" and "000000003cc56d0d0000000007fa48c5".

Conclusion:

Don't use it. It's not as useful as you might think and if you want to save memory by reusing objects, you can do it in a better (more performant, without global state and without the bugs!) way.

For example, your code is equivalent to

$product = Mage::getModel('catalog/product');
foreach($datas as $data) {
    $product->addData($data);
    $product->save();
}

The code you linked from Mage_Catalog_Model_Convert_Adapter_Product could as well have been:

    if (is_null($this->_productModel)) {
        $this->_productModel = Mage::getModel('catalog/product')
    }
    return $this->_productModel;

The key returned from the object cache is not used anywhere else, so why put it there to be globally available? The developer probably also thought there would be some magic optimization going on.

Interestingly enough, I don't find any usage in core code of the special features (cache tag, delete by model), only useless "save and load in the same place" code as in the example above.

So it looks like you found another case of historic code that was never really used.

3
  • Made several tests, seems to have a pretty good impact on performance though. So what do you think about the way Magento uses it internally : github.com/OpenMage/magento-mirror/blob/1.9.2.4/app/code/core/… ? Commented Sep 29, 2016 at 16:08
  • I think you don't understand yet what it does. See my update. Using the same object is faster than creating a new one, of course. But this comes with all the problems of using the same object. Commented Sep 29, 2016 at 16:09
  • 1
    What I think about this core code? It is stupid. Commented Sep 29, 2016 at 16:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.