Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:80059
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.220.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c9HYNH6AeqyrKcdNPSx_=Vi8jdx23wABVnZ3HkZ7BqF+g@mail.gmail.com>
References: <CA+9eiLsT72Cihvn7xYyQHLsD1vHO9Ef1yXyJQ=iOi=7WO9XZ5Q@mail.gmail.com>
	<CAF+90c9HYNH6AeqyrKcdNPSx_=Vi8jdx23wABVnZ3HkZ7BqF+g@mail.gmail.com>
Date: Thu, 1 Jan 2015 20:54:19 +0400
Message-ID: <CA+9eiLvGp5NgMeyeShrQkbhLVbCBv7jovTRZSi4XuTCWopu3og@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP Internals <internals@lists.php.net>, Xinchen Hui <xinchen.h@zend.com>
Content-Type: multipart/alternative; boundary=001a11333a4c499141050b9a131f
Subject: Re: Merge HashTable and zend_array into a single data structure.
From: dmitry@zend.com (Dmitry Stogov)

--001a11333a4c499141050b9a131f
Content-Type: text/plain; charset=UTF-8

Hi Nikita,
On Jan 1, 2015 5:23 PM, "Nikita Popov" <nikita.ppv@gmail.com> wrote:
>
> On Wed, Dec 31, 2014 at 11:19 AM, Dmitry Stogov <dmitry@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
>

--001a11333a4c499141050b9a131f--