Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:74150
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLuoJM5bwA46M_2YddTXYOhAwhii=9Ref7-NNeuRuBfW5A@mail.gmail.com>
References: <e5f19ac0089d24adc695fce93ad9bd9c.squirrel@webmail.klapt.com>
	<CA+9eiLuoJM5bwA46M_2YddTXYOhAwhii=9Ref7-NNeuRuBfW5A@mail.gmail.com>
Date: Wed, 14 May 2014 06:44:59 +0200
Message-ID: <CAEZPtU7zz=npjUhacwSBA6cz+kb9iBmn3fjkvYuAjqtiPQgJhw@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Anatol Belski <anatol.php@belski.net>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [VOTE] [RFC] 64 bit platform improvements for string
 length and integer
From: pierre.php@gmail.com (Pierre Joye)

hi Dmitry.

On Wed, May 14, 2014 at 12:52 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> Anatol,
>
> We discussed your patch in private and I showed you the big penalty it
> makes...

We discussed how we could best cooperate to get phpng and this patch
together to ease everyone's work.

> I really, don't see, what do you like to achieve initiating voting right
> after that. :(

Moving forward as you rejected the whole idea for no good reason or
based on numbers that cannot be taken as valid as this stage.

> I've just take a quick look over your initial patch for phpng at
> https://gist.github.com/weltling/a941d8cf6c731640b51f
>
> Actually, I would support only one idea from your patch - make IS_LONG to
> be 64-bit on _WIN64.

Again. This patch is about clean, safe, standard implementation for
64bit plaforms and modern compilers, by far not only for windows
(which has one more gain with this patch, portability). It brings a
lot of guards, as well as  safe and clean implementations. This is
something we should have done a long time ago. PHP is only of the only
OSS language I know still relying on old types and using integer for
buffer length. This is a bad practice and a well known troubles
source. To reduce it to only windows is hardly a good move.

> Your zend_size_t related changes, in my opinion, makes little sense and
> actually makes more harm. I recompiled phpng with your patch on Linux
> x86_64 and got the following numbers:
>
> zend_string size increased from 24 to 32 bytes
> HashTable size increased from 56 to 72 bytes
> zend_op_array size increased from 248 to 264 bytes
> zend_class_entry size increased from 512 to 568 bytes
> size of each opcode sizeof(zend_op) from 48 to 56 bytes

These numbers cannot be taken as valid or seriously at this stage.
Restructuring these structs will certainly reduce the delta. I did not
have the time to analyze phpng possible other improvements but I will
do it soon, and will also check with other people from the compiler
team if we can improve it a bit more as well.

But rejecting a safe and clean 64bit support for a prototype, even
promising, is wrong, in so many ways. See below.

> Anyone may recompile phpng (with and without patch) and then get these
> number running
> $ gdb sapi/cli/php
>> p sizeof(zend_string)
> ...
>
> Can you imagine memory consumption difference on a large application?
> More memory usage => more CPU cache misses => worse performance.

I can imagine a lot of things but at this point phpng is a very good
prototype with promising results. There is a good base idea and  a lot
of changes in many places improving performance. However it is very
far away from being ready, APIs are very inconsistent and painful to
use (mix of zend_string and char* usage, remaing or removal which may
not make sense or make the code way too complicated).

> and what are the advantages? strings and class names > 2GB?
> For me it's too big payment for useless feature.

I do not think it is that useful for both of us to redo the
discussions about this patch and its goal. It is a necessary step for
64bit support. It is also very hard to use a prototype, developed for
months privately and still being in very early pre-alpha/testing phase
as argument to reject these changes. However, as I told you, it would
be way better to do both together, for everyone. But you reject the
idea. We have to move forward to php-next and I do not think we can
afford to wait months until phpng is remotely usable or stable (at
least APIs wised).

In any case, if the 64bit patch is accepted we will support you and
other with phpng as we always do, even before its RFC or proposal,
this is what I call cooperation and teamwork.

Cheers.
Pierre