Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve PHP parser compatibility with different server configurations #1869

Merged
merged 5 commits into from
Jul 13, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jul 12, 2017

Fixes #1611.

The rest of WordPress does not depend on the presence of the mbstring extension or on the Unicode functionality of PCRE. After this PR, our PHP parser doesn't either.

As compared to #1775, the first set of changes here yields a slight performance improvement:

JavaScript :  11ms
PHP 5.6    : 135ms (down from 150ms)
PHP 7.0    :  17ms (no significant change)
PHP 7.1    :  16ms (down from 19ms)

Under normal circumstances, the second set of changes will have no performance impact. Forcing the code to use the alternative (non-PCRE-Unicode) method of splitting a string slows PHP 5.6 down by about 5ms, and does not appear to affect PHP 7 performance significantly.

For more details on the specific approaches taken here, see #1611 (comment).

This is a per-iteration limit rather than a total limit, so there's no
need to increase it from the value used in the rest of WP core.
@nylen nylen force-pushed the remove/php-parser-mbstring-calls branch from ffb2a1f to d40deff Compare July 12, 2017 14:57
@nylen nylen requested a review from mdawaffe July 12, 2017 15:04
Copy link
Contributor

@mdawaffe mdawaffe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tests pass :)

lib/compat.php Outdated
if ( count( $pieces ) > 1 ) {
$str = array_pop( $pieces );
} else {
$str = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be clearer to just break here, but I bow to your judgement.

@nylen nylen merged commit 15c520f into master Jul 13, 2017
@nylen nylen deleted the remove/php-parser-mbstring-calls branch July 13, 2017 11:02
nylen added a commit that referenced this pull request Jul 13, 2017
Tug pushed a commit that referenced this pull request Feb 28, 2020
…e-aztec-example-project

Delete react-native-aztec's example project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants