Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87590
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.15.19 as permitted sender)
Message-ID: <55C008B1.2050008@gmx.de>
Date: Tue, 04 Aug 2015 02:34:57 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Anatol Belski <anatol.php@belski.net>, 
 'Christoph Becker' <cmbecker69@gmx.de>,
 'Pierre Joye' <pierre.php@gmail.com>
CC: 'PHP internals' <internals@lists.php.net>
References: <55B0CADD.8000807@gmx.de> <CAEZPtU7aPzPQ=DL5fn9fOEuU_hySm5WXUkbjFw+Sq0T5xNfb3A@mail.gmail.com> <55B136BE.8010006@gmx.de> <046401d0c644$9cf76880$d6e63980$@belski.net> <55B2B78A.4080202@gmx.de> <048001d0c665$ed19ec40$c74dc4c0$@belski.net> <55B2D779.9090409@gmx.de> <04d301d0c6e5$6b28f0c0$417ad240$@belski.net> <55B3B2E6.7080905@gmx.de> <056e01d0c7d9$b1edd990$15c98cb0$@belski.net> <55B559C7.9060304@gmx.de> <064301d0c85f$5260ac60$f7220520$@belski.net>
In-Reply-To: <064301d0c85f$5260ac60$f7220520$@belski.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] PCRE JIT stack size limit
From: cmbecker69@gmx.de (Christoph Becker)

On 27.07.2015 at 13:28, Anatol Belski wrote:

>> -----Original Message-----
>> From: Christoph Becker [mailto:cmbecker69@gmx.de]
>> Sent: Monday, July 27, 2015 12:06 AM
>> To: Anatol Belski <anatol.php@belski.net>; 'Pierre Joye'
>> <pierre.php@gmail.com>
>> Cc: 'PHP internals' <internals@lists.php.net>
>> Subject: Re: [PHP-DEV] PCRE JIT stack size limit
>>
>> Definitely interesting ideas.  However, currently I'm leaning towards a full-
>> fledged solution in PHP 7.1 (whatever that may be), and to only add the error
>> constant for PHP 7.0.  Users can set pcre.jit=0 in php.ini and not change the
>> setting during runtime, if necessary.
>>
>> FWIW, some hours ago I've implemented the clean whole cache solution:
>> <https://github.com/cmb69/php-
>> src/commit/c41d78046da48db2fa8f5bd82fd6ac58099288f8>.
>
> I wrote this script to test it https://gist.github.com/weltling/cd7f8127378989cca87c , but as expected - cache reset make a significant impact when the cache is full. Though, maybe as it's an exceptional situation, it were the simplest solution. Because it does not affect the normal usage, maybe it's ok for 7.0. Maybe we should give this flexibility, still in doubt. But the constant IMHO should be done anyway.

IMHO we shouldn't introduce the cache clearing.  As your test script
shows, the performance might suffer considerably when switching pcre.jit
in a running script.  Furthermore cached regexps currently in use (e.g.
via preg_replace_callback) can't be cleared, what might lead to
confusion (in very rare cases, though).

As it is now users can set pcre.jit=0 and so will retain full
compatibility with PHP 5.  If they're running with pcre.jit=1, they're
able to get a meaningful error (PREG_JIT_STACKLIMIT_ERROR), if the
default stack is not large enough (what indeed might rarely be the case).

If we leave it as this, I suggest to make pcre.jit PHP_INI_PERDIR
instead of PHP_INI_ALL for PHP 7.0.  That still gives users the option
to disable it (ideally even on shared hosting), but avoids issues when
dynamically switching the setting with regard to our regexp caching.  If
we find a better solution for PHP 7.1, we still can extend it to
PHP_INI_ALL without a BC break.

For PHP 7.1 it might be worthwhile to consider to store jitted resp.
non-jitted regexps in the cache, and to fetch what fits to the current
setting of pcre.jit (and otherwise to compile again, adding the new
entry to the cache, or maybe replacing it).

-- 
Christoph M. Becker