Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83936
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.50 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <d238d736f8fcde9e649cb38c9b05fcc1@mail.gmail.com>
References: <d238d736f8fcde9e649cb38c9b05fcc1@mail.gmail.com>
Date: Fri, 27 Feb 2015 01:06:22 +0100
Message-ID: <CAF+90c_4tAoyy6SACu7c5Rw7_6airrwDJEzUAWXauvqFwQQC3A@mail.gmail.com>
To: Zeev Suraski <zeev@zend.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b3a834c7ece9d051006a38d
Subject: Re: [PHP-DEV] Coercive STH - some real world tests and updated RFC
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7b3a834c7ece9d051006a38d
Content-Type: text/plain; charset=UTF-8

On Fri, Feb 27, 2015 at 12:57 AM, Zeev Suraski <zeev@zend.com> wrote:

>  All,
>
> We've been working in the last few days to test and tune the Coercive STH
> patch.  I think the results are quite nice, and surprisingly better than
> one might have expected.
>
> Before diving into the results, we did update the RFC
> (wiki.php.net/rfc/coercive_sth) - with the most notable difference being
> allowing NULL->scalar conversions, for two reasons - it's not uncommon for
> code to use 'null' as a way to denote an empty optional parameter to for
> internal functions, and many internal functions seem to rely on that
> behavior themselves.  It should be possible to alter this behavior in the
> future by changing the internal functions to explicitly handle NULL inputs
> as 'please use the default value' - but it's outside the scope of the RFC.
>
> In addition, coercion from scalar to boolean is now limited only to
> integer - which seems to be the most popular use case; Beforehand,
> coercion was also allowed from float and string - but based on feedback
> from Mike, we're reconsidering accepting strings again.
>
> Another change being considered and not yet in the RFC is re-allowing
> leading and trailing spaces for numeric strings (sorry Paddy.)
>
> Now to the tests we ran.  The goal was to see what kind of effect the
> changes to the internal function APIs had on real world apps with large
> codebase.  The test methodology was done by placing a debugger breakpoint
> on zend_error, to ensure no error gets lost in the ether of settings or
> callbacks.  Here are the results:
>
>
> Drupal homepage:  One new E_DEPRECATED warning, which seems to catch a
> real bug, or at least faulty looking code:
>   $path = trim($path, '/');  // raises E_DEPRECATED, as $path is boolean
> false.
>   return $path;
>
> Drupal admin interface (across the all pages):  One  new E_DEPRECATED
> warning, which again seems to catch a real bug - stripslsahes() operating
> on a boolean.
>
> Magento homepage (w/ Magento's huge sample DB):  One new E_DEPRECATED
> warning, again, seems to be catching a real bug of 'false' being fed as
> argument 1 of in json_decode() - which is expecting a string full of json
> there.
>
> WordPress homepage:  One new E_DEPRECATED warning, again, seems to be
> catching a real bug of 'false' being fed as argument 1 of substr().
>
> Zend Framework 2 skeleton app:  Zero  new E_DEPRECATED warnings.
>
> Symfony ACME app:  Zero new E_DEPRECATED warnings (across the app).
>
>  As I'm sure you know, the above tests invoke a huge number of lines of
> code in total, handling filesystem ops, database ops and all sorts of
> other things.  This is much of the mini test suite that we use to test
> PHP's performance and compatibility (e.g. phpng, now PHP 7).  So while
> this isn't proof that code in the wild isn't going to have more issues -
> it's a pretty good initial indication regarding the level of 'breakage' we
> can expect.  I'm putting breakage in quotes, as people would have several
> years to fix these few issues, before the E_DEPRECATED becomes an error
> (or an exception, if the engine exceptions RFC passes).
>
> In terms of the test suite (.phpts), the changes cause approximately 700
> extra tests to fail out of 13,700, in comparison to w/o the patch.
> However, even though I didn't have a chance to go over all of them, it
> seems that the vast majority of the failed tests are tests that were
> intentionally designed to cover the exact parameter passing behavior,
> rather than real likely real world code pieces.   A huge number of the
> internal functions have this in their test suites:
>
> $variation = array(
>   'float 10.5' => 10.5,
>   'float -10.5' => -10.5,
>   'float 12.3456789000e10' => 12.3456789000e10,
>   'float -12.3456789000e10' => -12.3456789000e10,
>   'float .5' => .5,
>   );
>
> foreach ( $variation as $var ) {
>   var_dump(readgzfile( $filename, $var  ) );
> }
>
> Which clearly isn't a very likely input to the size argument of
> readgzfile().  All of these now trigger E_DEPRECATED warnings since we no
> longer allow float->int conversions if there's no data loss.  For some
> reason (perhaps a good one, not sure), we have virtually identical pieces
> of code for dozens if not hundreds of internal functions that expect
> integers - and these types of failures account for the (looks like vast)
> majority of phpt failures.  Quoting Andrea from a couple of days ago on
> this very topic, "to be fair, most of PHP's test suite basically just
> tests zpp's behavior", which appears to be absolutely true.  The
> real-world app tests are probably a much better indicator of the kind of
> behaviors our users are going to see than our test suite is.
>
> The takeaway from this seems to be that the approach of tightening the
> existing rule-set and applying it to both internal functions and new
> user-land code is viable, and does not cause the sky to come down falling.
> Preliminary tests suggest it actually finds real potential problems.
>
> More tomorrow.
>
> Zeev
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
How many deprecations do you get running the ZF2 and Symfony testsuites?

Nikita

--047d7b3a834c7ece9d051006a38d--