Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:74154
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain zend.com from 209.85.220.182 cause and error)
MIME-Version: 1.0
In-Reply-To: <CAEZPtU7zz=npjUhacwSBA6cz+kb9iBmn3fjkvYuAjqtiPQgJhw@mail.gmail.com>
References: <e5f19ac0089d24adc695fce93ad9bd9c.squirrel@webmail.klapt.com>
	<CA+9eiLuoJM5bwA46M_2YddTXYOhAwhii=9Ref7-NNeuRuBfW5A@mail.gmail.com>
	<CAEZPtU7zz=npjUhacwSBA6cz+kb9iBmn3fjkvYuAjqtiPQgJhw@mail.gmail.com>
Date: Wed, 14 May 2014 09:57:59 +0400
Message-ID: <CA+9eiLuDqLfkvBW2-Wa2T+BKeXVFhZnZ_SQ+MroWnyf4O07LaQ@mail.gmail.com>
To: Pierre Joye <pierre.php@gmail.com>
Cc: Anatol Belski <anatol.php@belski.net>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11334c38de96c704f955dc2c
Subject: Re: [PHP-DEV] [VOTE] [RFC] 64 bit platform improvements for string
 length and integer
From: dmitry@zend.com (Dmitry Stogov)

--001a11334c38de96c704f955dc2c
Content-Type: text/plain; charset=UTF-8

Pierre,

you developed a patch that will negatively affect every PHP application,
just to use "modern" types...
it's completely not necessary step for 64-bit support.

As I said, I can support only part of your patch related to IS_LONG size.

Thanks. Dmitry.



On Wed, May 14, 2014 at 8:44 AM, Pierre Joye <pierre.php@gmail.com> wrote:

> 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
>

--001a11334c38de96c704f955dc2c--