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

[5.3] Changed startsWith and endsWith to use substr (non mbstring) #15380

Merged
merged 2 commits into from
Sep 11, 2016
Merged

[5.3] Changed startsWith and endsWith to use substr (non mbstring) #15380

merged 2 commits into from
Sep 11, 2016

Conversation

Perturbatio
Copy link
Contributor

This is OK here because it doesn't

matter whether it's multi-byte unicode chars or not, just that the same number of bytes match at
the start or the end.

The performance of the existing methods becomes increasingly slow with larger strings due to
mbstring functions.

See https://gist.github.com/Perturbatio/adfe10304f65fb7bdd1adda0435c2f93 for a test.

Running this on my machine using an extreme example of 4.5MB text@10K iterations returns:

startsWith against validMatch
Finished in 130.06023406982 seconds, 10000 matches found

startsWith against invalidMatch
Finished in 141.01113009453 seconds, 0 matches found

startsWithAlt against validMatch
Finished in 0.0055170059204102 seconds, 10000 matches found

startsWithAlt against invalidMatch
Finished in 0.0040779113769531 seconds, 0 matches found

endsWith against validMatch
Finished in 221.22829198837 seconds, 10000 matches found

endsWith against invalidMatch
Finished in 221.26664710045 seconds, 0 matches found

endsWithAlt against validMatch
Finished in 0.0044331550598145 seconds, 10000 matches found

endsWithAlt against invalidMatch
Finished in 0.0062880516052246 seconds, 0 matches found

… ok here because it doesn't

matter whether it's multi-byte unicode chars or not, just that the same number of bytes match at
the start or the end.

The performance of the existing methods becomes increasingly slow with larger strings due to
 mbstring functions.
@GrahamCampbell GrahamCampbell changed the title Changed startsWith and endsWith to use substr (non mbstring) [5.3] Changed startsWith and endsWith to use substr (non mbstring) Sep 9, 2016
@taylorotwell taylorotwell merged commit 9007318 into laravel:5.3 Sep 11, 2016
@@ -403,7 +403,7 @@ public static function snake($value, $delimiter = '_')
public static function startsWith($haystack, $needles)
{
foreach ((array) $needles as $needle) {
if ($needle != '' && mb_strpos($haystack, $needle) === 0) {
if ($needle != '' && substr($haystack, 0, strlen($needle)) === $needle) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Perturbatio Why not just strpos($haystack, $needle) === 0 ?

Copy link
Contributor Author

@Perturbatio Perturbatio Sep 11, 2016

Choose a reason for hiding this comment

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

@vlakoff strpos only bails when it finds a match, in the case of a non-match it runs from the start to the end of the string, if you have a large string (i.e. 4.5MB as in my example) you have to check the entire string which is inefficient since we only care about the first strlen($needle) bytes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, great change then :)

One issue though, $needle should be cast to string. I'll make a PR for this tonight.

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.

None yet

3 participants