The death of `#ifndef FAST_ZPP`?

Hi everyone,

3 months and 6 days on, nobody has responded to this.

Will these chunks of dead code ever be removed?

Thanks!

Andrea Faulds wrote:

Hi everyone,

You may recall that PHP 7.0 introduced a new API for processing
arguments to internal functions, the "Fast Parameter Parsing API" or
FAST_ZPP (https://wiki.php.net/rfc/fast_zpp), which is an alternative to
the existing zend_parse_parameters function family.

Since FAST_ZPP was implemented, many functions in built-in PHP
extensions look like this:

#ifndef FAST_ZPP
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|S", &str, &what) ==
FAILURE) {
return;
}
#else
ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_STR(str)
Z_PARAM_OPTIONAL
Z_PARAM_STR(what)
ZEND_PARSE_PARAMETERS_END();
#endif

That is, they have /two/ sets of parameter processing code: one using
the old API, and one using the new API, with conditional compilation.
This is necessary because it is still possible to turn off FAST_ZPP by
changing the value of the macro.

However, should that be the case? The FAST_ZPP RFC was approved and PHP
7.0 was released containing it enabled by default, so it's no longer
just an experimental performance enhancement. Furthermore, I have no
reason to believe that the option to turn off FAST_ZPP is actually used
in practice, though feel free to prove me wrong!

Requiring all usages of FAST_ZPP to be accompanied by a
zend_parse_parameters() fallback creates code bloat. It also creates
hidden, potentially build-breaking bugs, because the code is rarely
built with FAST_ZPP disabled. It's like the old TSRMLS_CC situation.

Furthermore, for those who would prefer the FAST_ZPP API, it is
inconvenient that they cannot use it exclusively and must also use
zend_parse_parameters(), doubling the effort.

So, I would like to ask: would there be any objections to going through
and removing all unnecessary zend_parse_parameters fallbacks, and making
it impossible to disable FAST_ZPP? This would make the two APIs equally
valid.

Thanks!

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

Since FAST_ZPP was implemented, many functions in built-in PHP
extensions look like this:

[…]

That is, they have /two/ sets of parameter processing code: one using
the old API, and one using the new API, with conditional compilation.
This is necessary because it is still possible to turn off FAST_ZPP by
changing the value of the macro.

However, should that be the case? The FAST_ZPP RFC was approved and PHP
7.0 was released containing it enabled by default, so it's no longer
just an experimental performance enhancement.

It seems to me that the FAST_ZPP macro is not (only) to disable an
experimental feature, but rather to be able to reduce the size of the
built binaries. I've just checked a ./configure --disable-all:

with FAST_ZPP: 16,651,912
without FAST_ZPP: 16,276,152

That's a difference of 375,760 bytes, but only 2% increase.

Furthermore, I have no
reason to believe that the option to turn off FAST_ZPP is actually used
in practice, though feel free to prove me wrong!

Perhaps that is so, because FAST_ZPP can't be turned of via ./configure.

Requiring all usages of FAST_ZPP to be accompanied by a
zend_parse_parameters() fallback creates code bloat. It also creates
hidden, potentially build-breaking bugs, because the code is rarely
built with FAST_ZPP disabled. It's like the old TSRMLS_CC situation.

Furthermore, for those who would prefer the FAST_ZPP API, it is
inconvenient that they cannot use it exclusively and must also use
zend_parse_parameters(), doubling the effort.

Valid points.

So, I would like to ask: would there be any objections to going through
and removing all unnecessary zend_parse_parameters fallbacks, and making
it impossible to disable FAST_ZPP? This would make the two APIs equally
valid.

All in all, I think that the benefits of using only one of the parameter
parsing APIs per function outweigh the drawbacks, so I'm +1 on the
removal of the FAST_ZPP macro.

--
Christoph M. Becker