Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104972
MIME-Version: 1.0
References: <CAK5kH3x-P+eDXtf0wsm-=sffMq6y_6QGAdavikMEOGu=R1ddeA@mail.gmail.com>
 <CAK5kH3zGox7e9o3NhQav2hutk3hZoKAeXPkH7dNK9+aSDmbemw@mail.gmail.com>
 <CAF+90c89oR5OhC6ihAiSrQZtPfCEUStevKp8jg00HVE1gcpKDg@mail.gmail.com>
 <CAF+90c_0+eDvxuNi1NyMN=K39MtaNH7qXV=WrH5D0CB943VJng@mail.gmail.com>
 <CAK5kH3xk=xNR2+5McWVaOfo9_MRS=JydTpraSB1DpQXNjkvGUQ@mail.gmail.com>
 <CAF+90c_fvgMo+Et_RDK_P-fudVQO+ZGen=F5gGxvhRsy2ntkrQ@mail.gmail.com>
 <CAK5kH3yJuh-_vkpiBNjXyr-+E_zA+bFy=cZvEVcWSb3R4qLp7g@mail.gmail.com>
 <CAF+90c_O_FLw9Ms7s34+Lqv5CvMyNi8qWW9NKn9gnh99ash+Aw@mail.gmail.com>
 <CAK5kH3w3vvqGK0H_hYfjrKa+NUFeVSUVwcN4EE=sgsUtKtCyzw@mail.gmail.com>
 <CAF+90c8bgKD6QBnfG0ROiHYhD0o+FDZHfX1MQ2oTYwB8HM3F2Q@mail.gmail.com> <CAK5kH3y4roW4GY0t6RVaJQ2YSZW=0SJNSPpymH9JPJJeA94rQw@mail.gmail.com>
In-Reply-To: <CAK5kH3y4roW4GY0t6RVaJQ2YSZW=0SJNSPpymH9JPJJeA94rQw@mail.gmail.com>
Date: Wed, 27 Mar 2019 17:41:42 -0400
Message-ID: <CAK5kH3zQvzhi9QUtFWctaGHoWgN7dxpM3v0txS9yLQ30Dt430Q@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000005f7cc105851a4e0b"
Subject: Re: [PHP-DEV] Offset-only results from preg_match
From: cananian@wikimedia.org ("C. Scott Ananian")

--0000000000005f7cc105851a4e0b
Content-Type: text/plain; charset="UTF-8"

On Wed, Mar 27, 2019 at 2:30 PM C. Scott Ananian <cananian@wikimedia.org>
wrote:

> Continuing this saga: I'm still having performance problems on character
> entity expansion.  Here's the baseline code:
> https://github.com/wikimedia/remex-html/blob/master/RemexHtml/Tokenizer/Tokenizer.php#L881
> Of note: the regular expression is quite large -- around 26kB -- because
> it needs to include the complete table of all HTML5 entities, which it gets
> from a separate file of tables, HTMLData.php.
>
> Recapping briefly: we established before that it is very important that
> large regex strings been interned, otherwise pcre_get_compiled_regex_cache
> needs to do a full zend_string_equal_content() on every call to
> preg_match*, and since the strings will match, that costs a complete
> traversal of the 26kB regexp string.
>
> If I inline the char ref table directly into the regexp as a single huge
> literal string, that string is interned and (with Nikita's recent fixes for
> the CLI) things are ok.
>
> But that's bad for code maintainability; it violates Do Not Repeat
> Yourself and now it's much harder to see what the character reference
> regexp is doing because it's got this huge 26k table embedded in the middle
> of it.
>
> PHP will let me initialize the string as:
>
> const CHAR_REF_REGEXP = ' ... ' . HTMLData::NAMED_ENTITY_REGEX . "...";
>
> that is, it recognizes this as a compile-time constant -- but it doesn't
> actually intern the resulting string.  The code in
> zend_declare_class_constant_ex interns *most* constant strings, but in this
> case because there is a reference to another constant, the Z_TYPE_P(value)
> == IS_STRING check in zend_declare_class_constant_ex fails (the actual type
> is IS_CONSTANT_AST) presumably because we don't want to autoload HTMLData
> too soon. (But this also seems to happen even if I use
> self::NAMED_ENTITY_REGEX here, which wouldn't trigger the autoloader.)
>
> I *think* the proper fix is to intern the string lazily when it is finally
> evaluated, in ZEND_FETCH_CLASS_CONSTANT_SPEC_CONST_CONST_HANDLER around the
> point where we check Z_TYPE_P(value) == IS_CONSTANT_AST -- probably by
> tweaking zval_update_constant_ex to intern any string result?
>

I've created https://github.com/php/php-src/pull/3994 implementing this
fix, and confirmed that it is sufficient to get my large regexp interned
when it is rewritten as a class constant referencing
HTMLData::NAMED_ENTITY_REGEX.
 --scott

--0000000000005f7cc105851a4e0b--