[RFC] Fast Parameter Parsing API

Bob Weinand

12 years ago

Am 23.5.2014 um 13:36 schrieb Dmitry Stogov <dmitry@zend.com>:

> Please take a look at https://wiki.php.net/rfc/fast_zpp
>
> Thanks. Dmitry.

I just wanted to discuss here too, if we prefer:

for +: Z_PARAM_VARIADIC('+', dest, num)
for *: Z_PARAM_VARIADIC('*', dest, num)

or more:

for +: Z_PARAM_VARIADIC_EX(dest, num, minimum_necessary) (Set minimum_necessary to 1 for original '+' behaviour)
for *: Z_PARAM_VARIADIC(dest, num)

We weren't sure if the latter might confuse users of it, so...

I personally prefer the latter… The first is just inconsistent. We everywhere abolish these chars to indicate params and just there we leave them? Makes no sense to me...

Bob

Andrew Faulds

12 years ago

On 23 May 2014, at 13:23, Bob Weinand <bobwei9@hotmail.com> wrote:

> I personally prefer the latter… The first is just inconsistent. We everywhere abolish these chars to indicate params and just there we leave them? Makes no sense to me...

‘+’ and ‘*’ feel icky. One of the advantages of fast zpp is the nicer syntax that doesn’t feel like a regular expression. However, ‘+’ and ‘*’ feel very regexy. Definitely go with Z_PARAM_VARIADIC_EX. Maybe Z_PARAM_VARIADIC_MIN might be better.

--
Andrea Faulds
http://ajf.me/

Andrew Faulds

12 years ago

On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:

> Please take a look at https://wiki.php.net/rfc/fast_zpp
>
> Thanks. Dmitry.

This looks like a great idea. I like how it’s not only faster, but it seems to be more readable.

--
Andrea Faulds
http://ajf.me/

Johannes Schlueter

12 years ago

On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:

> On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
>
> > Please take a look at https://wiki.php.net/rfc/fast_zpp
> >
> > Thanks. Dmitry.
>
> This looks like a great idea. I like how it’s not only faster, but it
> seems to be more readable.

More readable? - I feel exactly the opposite. Yes, we get rid of void
pointers but use a pattern easy to understand. This looks like macro
hell.

If we break APIs anyways: Can't we go for C++ which allows providing
high level APIs with high performance ...

johannes

Pierre Joye

12 years ago

Hi Dmitry, Bob,

On Fri, May 23, 2014 at 3:28 PM, Johannes Schlüter
<johannes@schlueters.de> wrote:

> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
>> On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
>>
>> > Please take a look at https://wiki.php.net/rfc/fast_zpp
>> >
>> > Thanks. Dmitry.
>>
>> This looks like a great idea. I like how it’s not only faster, but it
>> seems to be more readable.
>
> More readable? - I feel exactly the opposite. Yes, we get rid of void
> pointers but use a pattern easy to understand. This looks like macro
> hell.

I feel the same way. I very much the idea of rewamping zpp, it is a
long due taks.

However the way it is implemented in this RFC is not something I would
like to work with for a decade or so. The performance gain is also not
sufficient enough to justify it.

I think we should think more about other options (no matter how long
you spent discussing options before posting this RFC). I am sure we
can come with other solutions which may be cleaner and also provide
some performance improvements.

> If we break APIs anyways: Can't we go for C++ which allows providing
> high level APIs with high performance ...

Yes, C++ would in this case provides almost all we need for
implementing parameter management in a clean and efficient way. Not
sure if it is desired tho'.

By the way, please do not ask me to come with code and cases to valid
my comment here, I won't. At least not until things get clarified
about we wish. The last thing we need is yet another Zend vs World
about that :)

Cheers,

--
Pierre

@pierrejoye | http://www.libgd.org

Dmitry Stogov

12 years ago

This patch is connected with Zend only in a way, that I worked on it during
last days (it's even not my idea).
but once I saw the idea I saw how it may be useful. I really don't
understand you attitude to us.
We are working hard to make a real big step forward...

2.5% speedup on real-life is not a good gain? where do you live? :)

Thanks. Dmitry.

On Fri, May 23, 2014 at 7:28 PM, Pierre Joye <pierre.php@gmail.com> wrote:

Pierre Joye

12 years ago

On May 23, 2014 5:58 PM, "Dmitry Stogov" <dmitry@zend.com> wrote:

>
> This patch is connected with Zend only in a way, that I worked on it

during last days (it's even not my idea).

> but once I saw the idea I saw how it may be useful. I really don't

understand you attitude to us.

> We are working hard to make a real big step forward...
>
> 2.5% speedup on real-life is not a good gain? where do you live? :)

In a world where clean code base and easy maintenance are better than a
couple % performance gain. :)

Dmitry Stogov

12 years ago

return to the land :)
PHP is used by millions people who never looked into its code.
We are working for them

BTW: 1% + 1% = 2%, and we already came to 30% :)
may be you may propose something that may increase performance at once?

Dmitry.


On Fri, May 23, 2014 at 8:15 PM, Pierre Joye <pierre.php@gmail.com> wrote:

Johannes Schlueter

12 years ago

On Fri, 2014-05-23 at 19:58 +0400, Dmitry Stogov wrote:

> This patch is connected with Zend only in a way, that I worked on it during
> last days (it's even not my idea).
> but once I saw the idea I saw how it may be useful. I really don't
> understand you attitude to us.
> We are working hard to make a real big step forward...

Thank you for working on it!

> 2.5% speedup on real-life is not a good gain? where do you live? :)

Yes I agree, 2.5% in real life is quite good.

"Nice" obviously is subjective thing and I agree that our role is to do
heavy lifting for our users. But what I'd wish is to try to reiterate a
round if we can find a "nicer" API. (I'll experiment, too, not sure I'll
have a "good" proposal)

johannes

Nuno Lopes

12 years ago

> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
>> On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
>>
>> > Please take a look at https://wiki.php.net/rfc/fast_zpp
>> >
>> > Thanks. Dmitry.
>>
>> This looks like a great idea. I like how it’s not only faster, but it
>> seems to be more readable.
>
> More readable? - I feel exactly the opposite. Yes, we get rid of void
> pointers but use a pattern easy to understand. This looks like macro
> hell.
>
> If we break APIs anyways: Can't we go for C++ which allows providing
> high level APIs with high performance ...

Totally agreed!
I strongly believe we should upgrade to C++.  All platforms already support
C++, and C++11 support is spreading quickly.
We wouldn't need to convert everything to C++ (classes and stuff). We could
start compiling with a C++ compiler and use C++ as needed and where it makes
sense. In particular we could use it to replace some macros in the Zend
engine.

For this particular example, with C++ you could improve performance of
parameter passing *without* changing the API at all. With templates one can
basically evaluate functions at compile time, and therefore most of the code
could be inlined (and let the "scanf" part be done at compile time).

C++ would also allow us to improve type safety and get rid of pointer
aliasing rules violations which will cause trouble sometime if not fixed.

Finally, I don't think the current proposal (the new macros) is very
readable. Moreover it requires substantial work to upgrade API users.

Nuno

P.S.: I've been a bit inactive in the past few years, but I'm really trilled
by the fact that phpng is taking momentum :)

Derick Rethans

12 years ago

On Fri, 23 May 2014, Johannes Schlüter wrote:

> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
> > On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
> >
> > > Please take a look at https://wiki.php.net/rfc/fast_zpp
> > >
> > > Thanks. Dmitry.
> >
> > This looks like a great idea. I like how it’s not only faster, but
> > it seems to be more readable.
>
> More readable? - I feel exactly the opposite. Yes, we get rid of void
> pointers but use a pattern easy to understand. This looks like macro
> hell.

I don't find it more readable either. From a single line, it goes to 7,
and what if there are three different zpp calls (like in
https://github.com/php/php-src/blob/master/ext/date/php_date.c#L4393) ?

I know the old API is not going away, but I'm not just sure about this
one...

> If we break APIs anyways: Can't we go for C++ which allows providing
> high level APIs with high performance ...

no :-)

cheers,
Derick

--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine

Dmitry Stogov

12 years ago

This API was designed to solve the existing bottleneck (5% CPU time lose).
It's not proposed to be used in all functions, only in few really affected.

Anyone may have their own opinion about readability (someones like Perl,
others Ada).

I appeal to think not about subjective readability, but about the cost of
ownership for @internals and PHP users.

Thanks. Dmitry.


On Wed, May 28, 2014 at 3:49 PM, Derick Rethans <derick@php.net> wrote:

Zeev Suraski

12 years ago

Are there specific functions you think we should use it that could give us
a 80/20 gain?

I didn’t think of it as an optimization we’d only use in ‘sensitive’
functions, but about wholesale migration from the current APIs to new ones.

Zeev

*From:* Dmitry Stogov [mailto:dmitry@zend.com]
*Sent:* Wednesday, May 28, 2014 3:13 PM
*To:* Derick Rethans
*Cc:* Johannes Schlüter; Andrea Faulds; PHP Internals; Bob Weinand; Stas
Malyshev; Zeev Suraski; Xinchen Hui; Andi Gutmans; Nikita Popov
*Subject:* Re: [PHP-DEV] [RFC] Fast Parameter Parsing API

This API was designed to solve the existing bottleneck (5% CPU time lose).

It's not proposed to be used in all functions, only in few really affected.

Anyone may have their own opinion about readability (someones like Perl,
others Ada).

I appeal to think not about subjective readability, but about the cost of
ownership for @internals and PHP users.

Thanks. Dmitry.

On Wed, May 28, 2014 at 3:49 PM, Derick Rethans <derick@php.net> wrote:

On Fri, 23 May 2014, Johannes Schlüter wrote:

> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
> > On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
> >
> > > Please take a look at https://wiki.php.net/rfc/fast_zpp
> > >
> > > Thanks. Dmitry.
> >
> > This looks like a great idea. I like how it’s not only faster, but
> > it seems to be more readable.
>
> More readable? - I feel exactly the opposite. Yes, we get rid of void
> pointers but use a pattern easy to understand. This looks like macro
> hell.

I don't find it more readable either. From a single line, it goes to 7,
and what if there are three different zpp calls (like in
https://github.com/php/php-src/blob/master/ext/date/php_date.c#L4393) ?

I know the old API is not going away, but I'm not just sure about this
one...

> If we break APIs anyways: Can't we go for C++ which allows providing
> high level APIs with high performance ...

no :-)

cheers,
Derick

--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine

Dmitry Stogov

12 years ago

~70 functions are in the patch.
they selected using profiler feedback from Wordpress home page.
Few functions are selected just to test rare argument specifiers (like 'C').
We may convert few more functions profiling other apps.

Thanks. Dmitry.

On Wed, May 28, 2014 at 4:24 PM, Zeev Suraski <zeev@zend.com> wrote:

Zeev Suraski

12 years ago

FWIW, if we use it only in a small subset of the functions that we deem
performance-sensitive, I think it makes a whole lot more sense.

I can think that we can live with the inflation in CAPS and lines if it’s
limited to just a few dozen functions.  It needs to be clear in the RFC
though.

Zeev

*From:* Dmitry Stogov [mailto:dmitry@zend.com]
*Sent:* Wednesday, May 28, 2014 3:40 PM
*To:* Zeev Suraski
*Cc:* Derick Rethans; Johannes Schlüter; Andrea Faulds; PHP Internals; Bob
Weinand; Stas Malyshev; Xinchen Hui; Andi Gutmans; Nikita Popov
*Subject:* Re: [PHP-DEV] [RFC] Fast Parameter Parsing API

~70 functions are in the patch.

they selected using profiler feedback from Wordpress home page.

Few functions are selected just to test rare argument specifiers (like 'C').

We may convert few more functions profiling other apps.

Thanks. Dmitry.

On Wed, May 28, 2014 at 4:24 PM, Zeev Suraski <zeev@zend.com> wrote:

Are there specific functions you think we should use it that could give us
a 80/20 gain?

I didn’t think of it as an optimization we’d only use in ‘sensitive’
functions, but about wholesale migration from the current APIs to new ones.

Zeev

*From:* Dmitry Stogov [mailto:dmitry@zend.com]
*Sent:* Wednesday, May 28, 2014 3:13 PM
*To:* Derick Rethans
*Cc:* Johannes Schlüter; Andrea Faulds; PHP Internals; Bob Weinand; Stas
Malyshev; Zeev Suraski; Xinchen Hui; Andi Gutmans; Nikita Popov
*Subject:* Re: [PHP-DEV] [RFC] Fast Parameter Parsing API

This API was designed to solve the existing bottleneck (5% CPU time lose).

It's not proposed to be used in all functions, only in few really affected.

Anyone may have their own opinion about readability (someones like Perl,
others Ada).

I appeal to think not about subjective readability, but about the cost of
ownership for @internals and PHP users.

Thanks. Dmitry.

On Wed, May 28, 2014 at 3:49 PM, Derick Rethans <derick@php.net> wrote:

On Fri, 23 May 2014, Johannes Schlüter wrote:

> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
> > On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
> >
> > > Please take a look at https://wiki.php.net/rfc/fast_zpp
> > >
> > > Thanks. Dmitry.
> >
> > This looks like a great idea. I like how it’s not only faster, but
> > it seems to be more readable.
>
> More readable? - I feel exactly the opposite. Yes, we get rid of void
> pointers but use a pattern easy to understand. This looks like macro
> hell.

I don't find it more readable either. From a single line, it goes to 7,
and what if there are three different zpp calls (like in
https://github.com/php/php-src/blob/master/ext/date/php_date.c#L4393) ?

I know the old API is not going away, but I'm not just sure about this
one...

> If we break APIs anyways: Can't we go for C++ which allows providing
> high level APIs with high performance ...

no :-)

cheers,
Derick

--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine

Bob Weinand

12 years ago

Again, we've already talked about the syntax a bit.

We have also other possibilities like:
zend_parse_parameters(Z_PARAM_ARRAY(input) Z_PARAM_LONG(offset) Z_PARAM_OPTIONAL Z_PARAM_ZVAL(z_length) Z_PARAM_BOOL(preserve_keys))
Or even shorter:
zend_parse_parameters(ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))
I think especially the latter is nicely readable and relatively short.

I updated the RFC with those above
And I've changed voting choices a bit:
https://wiki.php.net/rfc/fast_zpp#proposed_voting_choices

Am 28.5.2014 um 14:57 schrieb Zeev Suraski <zeev@zend.com>:

> FWIW, if we use it only in a small subset of the functions that we deem
> performance-sensitive, I think it makes a whole lot more sense.
>
> I can think that we can live with the inflation in CAPS and lines if it’s
> limited to just a few dozen functions.  It needs to be clear in the RFC
> though.
>
>
>
> Zeev
>
>
>
> *From:* Dmitry Stogov [mailto:dmitry@zend.com]
> *Sent:* Wednesday, May 28, 2014 3:40 PM
> *To:* Zeev Suraski
> *Cc:* Derick Rethans; Johannes Schlüter; Andrea Faulds; PHP Internals; Bob
> Weinand; Stas Malyshev; Xinchen Hui; Andi Gutmans; Nikita Popov
> *Subject:* Re: [PHP-DEV] [RFC] Fast Parameter Parsing API
>
>
>
> ~70 functions are in the patch.
>
> they selected using profiler feedback from Wordpress home page.
>
> Few functions are selected just to test rare argument specifiers (like 'C').
>
> We may convert few more functions profiling other apps.
>
> Thanks. Dmitry.
>
>
>
>
>
> On Wed, May 28, 2014 at 4:24 PM, Zeev Suraski <zeev@zend.com> wrote:
>
> Are there specific functions you think we should use it that could give us
> a 80/20 gain?
>
>
>
> I didn’t think of it as an optimization we’d only use in ‘sensitive’
> functions, but about wholesale migration from the current APIs to new ones.
>
>
>
> Zeev
>
>
>
> *From:* Dmitry Stogov [mailto:dmitry@zend.com]
> *Sent:* Wednesday, May 28, 2014 3:13 PM
> *To:* Derick Rethans
> *Cc:* Johannes Schlüter; Andrea Faulds; PHP Internals; Bob Weinand; Stas
> Malyshev; Zeev Suraski; Xinchen Hui; Andi Gutmans; Nikita Popov
> *Subject:* Re: [PHP-DEV] [RFC] Fast Parameter Parsing API
>
>
>
> This API was designed to solve the existing bottleneck (5% CPU time lose).
>
> It's not proposed to be used in all functions, only in few really affected.
>
> Anyone may have their own opinion about readability (someones like Perl,
> others Ada).
>
> I appeal to think not about subjective readability, but about the cost of
> ownership for @internals and PHP users.
>
> Thanks. Dmitry.
>
>
>
> On Wed, May 28, 2014 at 3:49 PM, Derick Rethans <derick@php.net> wrote:
>
> On Fri, 23 May 2014, Johannes Schlüter wrote:
>
>> On Fri, 2014-05-23 at 13:52 +0100, Andrea Faulds wrote:
>>> On 23 May 2014, at 12:36, Dmitry Stogov <dmitry@zend.com> wrote:
>>>
>>>> Please take a look at https://wiki.php.net/rfc/fast_zpp
>>>>
>>>> Thanks. Dmitry.
>>>
>>> This looks like a great idea. I like how it’s not only faster, but
>>> it seems to be more readable.
>>
>> More readable? - I feel exactly the opposite. Yes, we get rid of void
>> pointers but use a pattern easy to understand. This looks like macro
>> hell.
>
> I don't find it more readable either. From a single line, it goes to 7,
> and what if there are three different zpp calls (like in
> https://github.com/php/php-src/blob/master/ext/date/php_date.c#L4393) ?
>
> I know the old API is not going away, but I'm not just sure about this
> one...
>
>
>
>> If we break APIs anyways: Can't we go for C++ which allows providing
>> high level APIs with high performance ...
>
> no :-)
>
> cheers,
> Derick
>
> --
> http://derickrethans.nl | http://xdebug.org
> Like Xdebug? Consider a donation: http://xdebug.org/donate.php
> twitter: @derickr and @xdebug
> Posted with an email client that doesn't mangle email: alpine

Bob

Michael Wallner

12 years ago

On 28 May 2014 16:29, Bob Weinand <bobwei9@hotmail.com> wrote:

> Again, we've already talked about the syntax a bit.
>
> We have also other possibilities like:
> zend_parse_parameters(Z_PARAM_ARRAY(input) Z_PARAM_LONG(offset) Z_PARAM_OPTIONAL Z_PARAM_ZVAL(z_length) Z_PARAM_BOOL(preserve_keys))
> Or even shorter:
> zend_parse_parameters(ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))
> I think especially the latter is nicely readable and relatively short.

I like the (very) first proposed more than any other following.

--
Regards,
Mike

Michael Wallner

12 years ago

On 28 May 2014 14:13, Dmitry Stogov <dmitry@zend.com> wrote:

> This API was designed to solve the existing bottleneck (5% CPU time lose).
> It's not proposed to be used in all functions, only in few really affected.

Ah, now that's a noteworthy piece of information! :)

--
Regards,
Mike

Dmitry Stogov

12 years ago

My English, might be better, but it's probably not about English, but
possibility of misunderstanding in general.

The first sentence of proposal - "We propose an additional fast API for
parameter parsing, that should be used for the most useful functions".

Thanks. Dmitry.


On Wed, May 28, 2014 at 5:04 PM, Michael Wallner <mike@php.net> wrote:

Michael Wallner

12 years ago

On 28 May 2014 16:43, Dmitry Stogov <dmitry@zend.com> wrote:

> My English, might be better, but it's probably not about English, but
> possibility of misunderstanding in general.
>
> The first sentence of proposal - "We propose an additional fast API for
> parameter parsing, that should be used for the most useful functions".

Duh, right! I thought of it as "additional to the other changes in phpng" ;)

--
Regards,
Mike

Hannes Magnusson

12 years ago

On Fri, May 23, 2014 at 4:36 AM, Dmitry Stogov <dmitry@zend.com> wrote:

> Please take a look at https://wiki.php.net/rfc/fast_zpp

That.. Doesn't feel good.

Also, isn't it time to merge arginfo with zpp?
It seems very odd that these are two completely separate things we
need to maintain, and causes a lot of issues and weird things.

-Hannes

Pierre Joye

12 years ago

On May 23, 2014 7:02 PM, "Hannes Magnusson" <hannes.magnusson@gmail.com>
wrote:

>
> On Fri, May 23, 2014 at 4:36 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> > Please take a look at https://wiki.php.net/rfc/fast_zpp
>
>
> That.. Doesn't feel good.
>
> Also, isn't it time to merge arginfo with zpp?
> It seems very odd that these are two completely separate things we
> need to maintain, and causes a lot of issues and weird things.

I think so too. Maybe we can optimize it as well in a way to make
parameters mgt inside a function better and faster too.

Stas Malyshev

12 years ago

Hi!

> On Fri, May 23, 2014 at 4:36 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>> Please take a look at https://wiki.php.net/rfc/fast_zpp
>
>
> That.. Doesn't feel good.
>
> Also, isn't it time to merge arginfo with zpp?

That, btw, would be a very good idea if we ever want to have named
params. Because without proper arginfo named params can not work.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Xinchen Hui

12 years ago

Hey:

On Fri, May 23, 2014 at 7:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:

> Please take a look at https://wiki.php.net/rfc/fast_zpp

It's really a great improvement.

zend_parse_parameters is too heavy,  and to me:

Z_PARAM_STR()  is obviously readable than
zend_parse_parameter("s" /* what the hell s means here? */,)

and the performance gain is also very significant ..

actually, I thought about somethings like that.. but this patch
is better...

and these new APIs didn't break any APIS,   just provides a more
fast choice...

I really don't understand why you guys think it not good?

thanks

>
> Thanks. Dmitry.

--
Xinchen Hui  Laruence
Consultant    at   Zend
http://www.laruence.com/

Dmitry Stogov

12 years ago

I would be glad to see proposals from Hannes and Johannes, but we also need
to move phpng forward, and I wouldn't like to spend on this too much time.

Our proposal is mainly about speed (as well as the main phpng goal).
We identified yet another bottleneck and eliminated it.
Changes in readability and compile-time type checking are side effects of
implementation.

Thanks. Dmitry.


On Sat, May 24, 2014 at 4:50 PM, Xinchen Hui <xinchen.h@zend.com> wrote:

Michael Wallner

12 years ago

Hey Dmitry!

On 26 May 2014 07:39, Dmitry Stogov <dmitry@zend.com> wrote:

> I would be glad to see proposals from Hannes and Johannes, but we also need
> to move phpng forward, and I wouldn't like to spend on this too much time.
>
> Our proposal is mainly about speed (as well as the main phpng goal).
> We identified yet another bottleneck and eliminated it.
> Changes in readability and compile-time type checking are side effects of
> implementation.

Oh well... while I pretty much appreciate the efforts put into phpng,
Zend isn't all too popular for designing usable APIs... because it
often happens in a rush. Let me recall you that this list is often
about politics nowadays, and from a political POV this statement just
costed 20% of yay-votes.

--
Regards,
Mike

Pierre Joye

12 years ago

On Mon, May 26, 2014 at 8:25 AM, Michael Wallner <mike@php.net> wrote:

> Hey Dmitry!
>
> On 26 May 2014 07:39, Dmitry Stogov <dmitry@zend.com> wrote:
>> I would be glad to see proposals from Hannes and Johannes, but we also need
>> to move phpng forward, and I wouldn't like to spend on this too much time.
>>
>> Our proposal is mainly about speed (as well as the main phpng goal).
>> We identified yet another bottleneck and eliminated it.
>> Changes in readability and compile-time type checking are side effects of
>> implementation.
>
> Oh well... while I pretty much appreciate the efforts put into phpng,
> Zend isn't all too popular for designing usable APIs... because it
> often happens in a rush. Let me recall you that this list is often
> about politics nowadays, and from a political POV this statement just
> costed 20% of yay-votes.

Well, politics are one part of the problem and I agree that we see
that too often.

However what I see now is partially what I have seen back to php4 > 5
move. We are deciding now what we will use for a decade+ after the
release of php6. And as much as I like to get a faster PHP, I am
really looking forward (and do it) an API cleanup and other deeper
change to ease:

- core maintenance
- easier, well documented, exposed APIs for 3rd party developers (extensions)

It is no secret that our internals APIs are bad. PHP-CPP success
despite its current limitation or stability is yet another sign. I
have been working on Ruby, Go or Python internals lately, we have a
long road to achieve only part of the simplicity we can see there.
This is something I would like to put more efforts, first, prior to
push even more hacks to gain 1-2% (even if 10x 1% means 10%
performance increase). It is always easier to optimize a clean,
smaller code base, with less warnings, aliases, etc. than what we
actually have now.It is getting worst now because of partial changes
in many areas, simply because "it makes php faster if we change this
specific function".

I hope I do not make my point unclear, I appreciate a lot the
performance improvements Nikita, Laruence, Bob and Dmitry are working
on, this is very promising. However I would really like to focus on
global design, clean code base and as much as possible warning free as
well, this will make everyone work much easier for the next 10-12
years.

Cheers,

--
Pierre

@pierrejoye | http://www.libgd.org

Zeev Suraski

12 years ago

To be perfectly honest, I tend to agree that the readability issues with
this proposal make it a bit difficult to digest.  And I'm not taking a 2.5%
real world performance gain lightly - it's huge.

I think we need to consider other alternatives as code readability and
maintainability are important factors to consider, not just performance...

Zeev

On 26 במאי 2014, at 08:39, Dmitry Stogov <dmitry@zend.com> wrote:

I would be glad to see proposals from Hannes and Johannes, but we also need
to move phpng forward, and I wouldn't like to spend on this too much time.

Our proposal is mainly about speed (as well as the main phpng goal).
We identified yet another bottleneck and eliminated it.
Changes in readability and compile-time type checking are side effects of
implementation.

Thanks. Dmitry.


On Sat, May 24, 2014 at 4:50 PM, Xinchen Hui <xinchen.h@zend.com> wrote:

Pierre Joye

12 years ago

On Mon, May 26, 2014 at 9:01 AM, Zeev Suraski <zeev@zend.com> wrote:

> To be perfectly honest, I tend to agree that the readability issues with
> this proposal make it a bit difficult to digest.  And I'm not taking a 2.5%
> real world performance gain lightly - it's huge.
>
> I think we need to consider other alternatives as code readability and
> maintainability are important factors to consider, not just performance...

Merci! It is a pleasure to read that from you, I really mean it :-)

--
Pierre

@pierrejoye | http://www.libgd.org

Dmitry Stogov

12 years ago

I personally see it more readable than "sZ|l", but of course, other people
may think differently.
I would be glad if someone would propose something useful in reasonable
time.

Thanks. Dmitry.

On Mon, May 26, 2014 at 11:01 AM, Zeev Suraski <zeev@zend.com> wrote:

Zeev Suraski

12 years ago

I think the main challenge is the fact it turns what today is a one-liner
(albeit a bit obscure) into a long multi-line all-caps gig.   I’ve tried to
think about alternatives but so far came short with something that
elegantly solves the problem, short of using C++ which I think goes too far
in the context of what we’re trying to achieve.

I’ll do some more thinking…

Zeev

*From:* Dmitry Stogov [mailto:dmitry@zend.com]
*Sent:* Monday, May 26, 2014 10:10 AM
*To:* Zeev Suraski
*Cc:* Xinchen Hui; PHP Internals; Bob Weinand; Stas Malyshev; Andi Gutmans;
Nikita Popov
*Subject:* Re: [RFC] Fast Parameter Parsing API

I personally see it more readable than "sZ|l", but of course, other people
may think differently.

I would be glad if someone would propose something useful in reasonable
time.

Thanks. Dmitry.

On Mon, May 26, 2014 at 11:01 AM, Zeev Suraski <zeev@zend.com> wrote:

To be perfectly honest, I tend to agree that the readability issues with
this proposal make it a bit difficult to digest.  And I'm not taking a 2.5%
real world performance gain lightly - it's huge.

I think we need to consider other alternatives as code readability and
maintainability are important factors to consider, not just performance...

Zeev

On 26 במאי 2014, at 08:39, Dmitry Stogov <dmitry@zend.com> wrote:

I would be glad to see proposals from Hannes and Johannes, but we also need
to move phpng forward, and I wouldn't like to spend on this too much time.

Our proposal is mainly about speed (as well as the main phpng goal).
We identified yet another bottleneck and eliminated it.
Changes in readability and compile-time type checking are side effects of
implementation.

Thanks. Dmitry.

On Sat, May 24, 2014 at 4:50 PM, Xinchen Hui <xinchen.h@zend.com> wrote:

Hey:

On Fri, May 23, 2014 at 7:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:

> Please take a look at https://wiki.php.net/rfc/fast_zpp

It's really a great improvement.

zend_parse_parameters is too heavy,  and to me:

Z_PARAM_STR()  is obviously readable than
zend_parse_parameter("s" /* what the hell s means here? */,)

and the performance gain is also very significant ..

actually, I thought about somethings like that.. but this patch
is better...

and these new APIs didn't break any APIS,   just provides a more
fast choice...

I really don't understand why you guys think it not good?

thanks

>
> Thanks. Dmitry.

--
Xinchen Hui  Laruence
Consultant    at   Zend
http://www.laruence.com/

Pierre Joye

12 years ago

On Mon, May 26, 2014 at 9:17 AM, Zeev Suraski <zeev@zend.com> wrote:

> I think the main challenge is the fact it turns what today is a one-liner
> (albeit a bit obscure) into a long multi-line all-caps gig.   I’ve tried to
> think about alternatives but so far came short with something that
> elegantly solves the problem, short of using C++ which I think goes too far
> in the context of what we’re trying to achieve.
>
>
>
> I’ll do some more thinking…

One thing I did not look at yet (too many things to catch up with :)
is how we could solve two problems in one shot.

Hannes and other mentioned the pain it is to maintain the parameters
information in two or three places, reflection structs, inline doc and
zpp. It could be possible to use the reflection struct for argument
parsing, while adding support for types and maybe named arguments
while being at it. Just a thought, I have to investigate this idea, or
anyone else with some more available time :)

Cheers,

--
Pierre

@pierrejoye | http://www.libgd.org

Andrew Faulds

12 years ago

On 26 May 2014, at 08:17, Zeev Suraski <zeev@zend.com> wrote:

> I think the main challenge is the fact it turns what today is a one-liner
> (albeit a bit obscure) into a long multi-line all-caps gig.   I’ve tried to
> think about alternatives but so far came short with something that
> elegantly solves the problem, short of using C++ which I think goes too far
> in the context of what we’re trying to achieve.

Perhaps it could be simplified? Something like this:

ZEND_PARSE_PARAMETERS(Z_PARAM_ARRAY, input, Z_PARAM_LONG, offset, Z_PARAM_OPTIONAL, Z_PARAM_ZVAL, z_length, Z_PARAM_BOOL, preserve_keys);

That’s still rather unwieldy, though. Maybe Z_PARAM could be shortened to ZP_? Then we’d have this:

ZEND_PARSE_PARAMETERS(ZP_ARRAY, input, ZP_LONG, offset, ZP_OPTIONAL, ZP_ZVAL, z_length, ZP_BOOL, preserve_keys)

That’s not so bad, right? That would be parsing at runtime instead of with macros, though. Maybe this?

ZEND_PARSE_PARAMETERS(ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys))

Except, at least if we’re only using C89, we can’t use a nice function-like macro syntax here. You’d also still need to explicitly state numbers of arguments. The first and second proposals are nice because they requires none of this due to runtime parsing, but it would be no faster than zend_parse_parameters.

Does anyone have a good idea?

--
Andrea Faulds
http://ajf.me/

Andrew Faulds

12 years ago

On 26 May 2014, at 09:24, Andrea Faulds <ajf@ajf.me> wrote:

> Except, at least if we’re only using C89, we can’t use a nice function-like macro syntax here.

I take this bit back, actually. You could do some horrible #define hack here. I’d rather not do that, though. So out with the commas it should probably be.

--
Andrea Faulds
http://ajf.me/

Michael Wallner

12 years ago

On 26 May 2014 10:24, Andrea Faulds <ajf@ajf.me> wrote:

> Perhaps it could be simplified? Something like this:
>
> ZEND_PARSE_PARAMETERS(Z_PARAM_ARRAY, input, Z_PARAM_LONG, offset, Z_PARAM_OPTIONAL, Z_PARAM_ZVAL, z_length, Z_PARAM_BOOL, preserve_keys);
>
> That’s still rather unwieldy, though. Maybe Z_PARAM could be shortened to ZP_? Then we’d have this:
>
> ZEND_PARSE_PARAMETERS(ZP_ARRAY, input, ZP_LONG, offset, ZP_OPTIONAL, ZP_ZVAL, z_length, ZP_BOOL, preserve_keys)
>
> That’s not so bad, right? That would be parsing at runtime instead of with macros, though. Maybe this?
>
> ZEND_PARSE_PARAMETERS(ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys))
>

WAT? Why does that look better to you than a) what we already have,
and b) what Dmitry proposed?

--
Regards,
Mike

Dmitry Stogov

12 years ago

I more or less like the last option, and actually it's similar to Bob
original proposal.
But I see two disadvantages:
1) It'll relay on macros with variable arguments and it may be not portable
across all compilers.
2) It uses nested macros and it may make a nightmare for finding source of
some syntax mistake in user code.

Thanks. Dmitry.


On Mon, May 26, 2014 at 12:24 PM, Andrea Faulds <ajf@ajf.me> wrote:

Andrew Faulds

12 years ago

On 26 May 2014, at 09:43, Dmitry Stogov <dmitry@zend.com> wrote:

> I more or less like the last option, and actually it's similar to Bob
> original proposal.
> But I see two disadvantages:
> 1) It'll relay on macros with variable arguments and it may be not portable
> across all compilers.
> 2) It uses nested macros and it may make a nightmare for finding source of
> some syntax mistake in user code.

It could be done without variable arguments, with different syntax. Actually, I’m not sure that syntax can be done *with* them based on how inflexible C99’s variadic macros are.

But the following could work, right?

ZEND_PARSE_PARAMETERS(2, 4, ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))

The syntax is ugly, though. It looks like it should be a function and have commas between arguments, but it doesn’t. You could do something like this:

ZEND_PARSE_PARAMETERS(2, 4, (ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys)))

That doesn’t require variadic macros as it would exploit the way the C preprocessor works. However, I’m not sure that’s actually an improvement, especially because the ZP_* macros would now look horrible, having to contain garbage at the beginning and end to cancel out the comma. It’s arguably uglier than the one above it.

If we go for the “Simpler Variation” proposal and don’t specify counts, then this is feasible:

ZEND_PARSE_PARAMETERS((ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys)))

But again, the doubly-nested () syntax is ugly and probably confusing. I’d rather go with the first option, even if it does look a little too much like a function. With the “Simpler Variation” proposal, it could even look like this:

ZEND_PARSE_PARAMETERS(ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))

Doesn’t that look nice? :)

--
Andrea Faulds
http://ajf.me/

Bob Weinand

12 years ago

> Am 26.05.2014 um 11:03 schrieb "Andrea Faulds" <ajf@ajf.me>:
>> On 26 May 2014, at 09:43, Dmitry Stogov <dmitry@zend.com> wrote:
>> I more or less like the last option, and actually it's similar to Bob
>> original proposal.
>> But I see two disadvantages:
>> 1) It'll relay on macros with variable arguments and it may be not portable
>> across all compilers.
>> 2) It uses nested macros and it may make a nightmare for finding source of
>> some syntax mistake in user code.
>
> It could be done without variable arguments, with different syntax. Actually, I’m not sure that syntax can be done *with* them based on how inflexible C99’s variadic macros are.
>
> But the following could work, right?
>
> ZEND_PARSE_PARAMETERS(2, 4, ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))
>
> The syntax is ugly, though. It looks like it should be a function and have commas between arguments, but it doesn’t. You could do something like this:
>
> ZEND_PARSE_PARAMETERS(2, 4, (ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys)))
>
> That doesn’t require variadic macros as it would exploit the way the C preprocessor works. However, I’m not sure that’s actually an improvement, especially because the ZP_* macros would now look horrible, having to contain garbage at the beginning and end to cancel out the comma. It’s arguably uglier than the one above it.
>
> If we go for the “Simpler Variation” proposal and don’t specify counts, then this is feasible:
>
> ZEND_PARSE_PARAMETERS((ZP_ARRAY(input), ZP_LONG(offset), ZP_OPTIONAL, ZP_ZVAL(z_length), ZP_BOOL(preserve_keys)))
>
> But again, the doubly-nested () syntax is ugly and probably confusing. I’d rather go with the first option, even if it does look a little too much like a function. With the “Simpler Variation” proposal, it could even look like this:
>
> ZEND_PARSE_PARAMETERS(ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys))
>
> Doesn’t that look nice? :)
> --
> Andrea Faulds
> http://ajf.me/
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

Actually, that's exactly my the original API, just with ZP_OPTIONAL instead of numbers for counts:

ZEND_PARSE_PARAMETERS(ZP_ARRAY(input) ZP_LONG(offset) ZP_OPTIONAL ZP_ZVAL(z_length) ZP_BOOL(preserve_keys), { return; })
(The return at the end is the error branch)

I still prefer it that way, but Dmitry doesn't...

It's the most readable I still think.
That's for me more important tgan being able to debug the macro. (Compiler still is useful for debugging wrong usage of macros here)

Bob Weinand (iPhone)