Skip to content

Port strtr from zend 5.6.10#5517

Closed
ebernhardson wants to merge 1 commit intofacebook:masterfrom
ebernhardson:port-strtr
Closed

Port strtr from zend 5.6.10#5517
ebernhardson wants to merge 1 commit intofacebook:masterfrom
ebernhardson:port-strtr

Conversation

@ebernhardson
Copy link
Contributor

Addresses #5471

strtr() invoked with an array $replace_pairs as the second argument appears to
perform much worse under HHVM than PHP5.

PHP's implementation of multiple-string search was formerly quite naïve, and it
was the subject of PHP bug 63893. In 2013, it was rewritten to utilize a clever
and efficient algorithm for multi-pattern searching (Wu-Manber), and then
further optimized. It looks like HHVM's implementation is based on the naive
implementation PHP sported before PHP 5.4, which explains the poor performance.

This patch ports over the strtr implementation in php 5.6.10.

@facebook-github-bot
Copy link
Contributor

This pull request has been imported into Phabricator, and discussion and review of the diff will take place at https://reviews.facebook.net/D40473

@ebernhardson ebernhardson force-pushed the port-strtr branch 3 times, most recently from 9160446 to 9ce1122 Compare July 6, 2015 15:36
@ebernhardson ebernhardson force-pushed the port-strtr branch 2 times, most recently from cda3307 to 7f982d7 Compare July 29, 2015 16:41
@ebernhardson ebernhardson force-pushed the port-strtr branch 3 times, most recently from 10deab1 to 2427448 Compare August 10, 2015 22:38
strtr() invoked with an array $replace_pairs as the second argument appears to
perform much worse under HHVM than PHP5.

PHP's implementation of multiple-string search was formerly quite naïve, and it
was the subject of PHP bug 63893. In 2013, it was rewritten to utilize a clever
and efficient algorithm for multi-pattern searching (Wu-Manber), and then
further optimized. It looks like HHVM's implementation is based on the naive
implementation PHP sported before PHP 5.4, which explains the poor performance.

This patch ports over the strtr implementation in php 5.6.10. This implementation
prefers the existing strtr_fast algorithm and falls back to wu-manber when there
are too many patterns.
@jwatzman
Copy link
Contributor

@ebernhardson can you have a look at #6384 please? It seems likely this change is the cause of that crash. I'm going to spend some time later to confirm, but if it is, we'll have to revert it if you don't send a PR soonish to fix it.

@jwatzman
Copy link
Contributor

#6395 is a proposed fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants