Hi Nikita, On Jan 1, 2015 5:23 PM, "Nikita Popov" <nikita....@gmail.com> wrote: > > On Wed, Dec 31, 2014 at 11:19 AM, Dmitry Stogov <dmi...@zend.com> wrote: >> >> Hi, >> >> Please take a look into the patch >> >> https://github.com/php/php-src/pull/970/files >> >> This real changes are in zend_types.h, the rest is renaming that in most cases makes code cleaner. >> >> zend_array didn't change its binary representation, but now it's not possible to get a pointer to embedded HashTable. The same zend_array shoukd be used instead. >> >> Each HashTable got an extra 64-bit zend_refcounted header. This leads to some increase in memory consumption. > > > In your patch zend_hash_init (and zend_hash_destroy as well) ignores the refcounted header. So currently you wouldn't be able to use just any HashTable* in a refcounted way (e.g. do an object->array cast by adding a ref to the properties HT and sticking it in a zval). You can only do the reverse, i.e. use a no longer used refcounted array somewhere else (like the way properties are assigned in pdo/... now). > > What's the plan about this? Is this intentional or will the hash API start managing the refcount as well (like strings already do)?
you are right. It must be fixed in some way. we may start playing adding assert(refcount == 1) in all zend_hash functions that modifyes HashTables. Other ideas are wecome... > >> >> The performance is slightly increased (may be measured with callgrind). >> >> The patch beaks one test (tests/lang/foreachLoopObjects.006.phpt), but actually it just disclose a problem that we have anyway. >> >> The patch should be a base for the future optimizations. e.g. removing HashTable->arData and/or HashTable->arHash and allocating them together with zend_array; introducing EG(empty_array) etc. > > > Could you elaborate on the first part? How is it possible to alloc arData/arHash together with zend_array? (Or rather, if you do that, how can you change the size later?) We may reallocate zend_array itself. Of course, it would require using zend_array ** in all functions that may modify it, and also using zend_array* instead of embeded HashTables. Thanks. Dmitry. > > Nikita >
