Split pcre cache into permanent and per-request caches #4004
Conversation
cscott
force-pushed
the
split-pcre-cache
branch
from
20dbfda
to
a5214d5
Compare
|
@cscott could you fix the merge conflicts please ? |
|
@cscott, any news? |
In addition to the permanent pcre cache, maintain a separate per-request cache so that we can use string identity comparisons instead of having to do full zend_string_equal_content() on every cache fetch -- this can be quite costly if the regular expression is large.
|
Required conflict resolution after e06836a was merged, but the changes required were straightforward. |
|
Still waiting on review. The test failure above appears to be spurious ("Bug #72333: fwrite() on non-blocking SSL sockets doesn't work [C:\projects\php-src\ext\openssl\tests\bug72333.phpt]" which has nothing to do with this patch as far as I can tell). |
|
@cscott @nikic This patch may make improvement only for some particular cases, when during single request, some regular expression string is constructed and then used many times. In other cases, for example, when regular expression string is reconstructed, the patch only makes slowdown because of additional overhead. In current state, I see slight "slowdown" on WordPress. Probably, this overhead may be reduced. I'll try to think a bit more about this problem. @scott could you provide a benchmark script, for the case you are optimizing. |
|
Closing as this has gone stale. Please reopen if you'd like to pick this back up with some benchmarks. |
In addition to the permanent pcre cache, maintain a separate per-request cache so that we can use string identity comparisons instead of having to do full zend_string_equal_content() on every cache fetch -- this can be quite costly if the regular expression is large.
This is an alternative to PR #3994 and could be considered a follow-up to PR #3985.
Uses the pcre_cache_entry refcount field to ensure that the permanent cache isn't cleaned while there is still a pointer to the entry in the request cache, but this cleaning mechanism could be further tuned.