Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:89727
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 68.114.190.31 cause and error)
Message-ID: <D509D561CF6B444BA5C56F591DCA3084@pc1>
To: <internals@lists.php.net>,
	"Andrea Faulds" <ajf@ajf.me>
References: <5F562492D25B4650AD21E7B51AD3C6E5@pc1> <77.28.28320.361E5665@pb1.pair.com>
Date: Mon, 7 Dec 2015 15:18:44 -0600
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=response
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: Compilation fails without FAST_ZPP
From: php_lists@realplain.com ("Matt Wilmas")

Hi,

----- Original Message ----
From: "Andrea Faulds"
Sent: Monday, December 07, 2015

> Hi Matt,
>
> Matt Wilmas wrote:
>> Hi Bob, all,
>>
>> After this commit:
>> http://git.php.net/?p=php-src.git;a=commitdiff;h=509712c7d9056b4ceb50134bfeea1a1115720744
>>
>>
>> In streamsfuncs.c, line 996 has an extra comma; and line 1511 has #ifdef
>> instead of #ifndef ...
>>
>>
>> BTW, maybe with the changes I'll propose, all this extra FAST_ZPP stuff
>> can be cleaned up, for 7.1 at least.
>
> Is it worth keeping these FAST_ZPP #ifdefs around, still? FAST_ZPP is 
> probably here to stay, and what was the core (not the entirety) of 
> FAST_ZPP is now always built regardless of whether the macro is defined, 
> because it shares parameter-handling code with zend_parse_parameters and 
> scalar type declarations.

Hold up on changes to the FAST_ZPP parts at this point! :-)  Like I [have] 
said, my changes are related and address that.  Sadly, the patch was 
supposed to be ready 3 months ago.  It's in the middle of beneficial 
refinement for the umpteenth time (first since summer, really :-D), and 
maybe it'll show up in a week or so *this* time.

Anyway, I was under the impression that the 2 #ifdef'd versions were for 
comparing the 2 methods (which is what I was doing), or if, even now, the 
inline parameter parsing wanted to be disabled, for smaller code size...

Besides the call sites, there's still stuff in zend_API.[ch] that's 
conditionally compiled...

My inline (FAST_ZPP alternative) solution can have the inline parsing part 
(which I'm trying to make as small as possible) disabled, and use a new 
zend_fast_parse_parameters(), or whatever it's called (also used with 
inlining if param type is wrong).  So, inlining or smallest [generated] code 
can be chosen with the same source.

A set of always_inline helper functions will also convert the old 
zend_parse_parameters functions to use the new FAST method (with all common 
compilers), without runtime string specifier parsing...  Generate code size 
<= current calls.

Since at least one new function is needed, not sure what could be changed in 
7.0 (I have some ideas), although there's also a faster function with 
runtime string parsing that IS fully compatible with 7.0.

> One problem with keeping the #ifdefs is that the FAST_ZPP and 
> zend_parse_parameters versions of argument handling can and do go out of 
> sync. Also, I don't think it's even an actual compile-time option to 
> disable FAST_ZPP anyway. You have to modify the source code and comment 
> out the #define.

Yep and yep and yep.

> So, would there be any objection to getting rid of the #else branches in 
> such #ifdefs?
>
> Thanks!

- Matt