Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:80043
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLsT72Cihvn7xYyQHLsD1vHO9Ef1yXyJQ=iOi=7WO9XZ5Q@mail.gmail.com>
References: <CA+9eiLsT72Cihvn7xYyQHLsD1vHO9Ef1yXyJQ=iOi=7WO9XZ5Q@mail.gmail.com>
Date: Thu, 1 Jan 2015 14:23:27 +0100
Message-ID: <CAF+90c9HYNH6AeqyrKcdNPSx_=Vi8jdx23wABVnZ3HkZ7BqF+g@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Xinchen Hui <xinchen.h@zend.com>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=f46d0437494128f484050b9721ff
Subject: Re: Merge HashTable and zend_array into a single data structure.
From: nikita.ppv@gmail.com (Nikita Popov)

--f46d0437494128f484050b9721ff
Content-Type: text/plain; charset=UTF-8

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)?


> 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?)

Nikita

--f46d0437494128f484050b9721ff--