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

[6.x] Optimize Service Provider registration #30960

Merged
merged 1 commit into from Dec 28, 2019
Merged

Conversation

@36864
Copy link
Contributor

36864 commented Dec 27, 2019

As per the discussion in #30952 and according to my tests, strpos is considerably faster than calling startsWith for short haystacks. This function is called for every provider on boot.
A default laravel installation will load 25 service providers.

I adapted the benchmark to test checking the default laravel packages. Benchmark code and results here: https://gist.github.com/36864/4bc70175ad0bb60b3261d1100f249cf5

Class names are unlikely to be longer than 100 characters, so this change should not have a negative impact.

According to the same benchmarks, a direct substring comparison would also be faster than the current implementation, but slower than strpos for strings

Sample result, over 1000000 iterations, checking if the provider class name starts with Illuminate\:

Provider: Illuminate\Foundation\Providers\ConsoleSupportServiceProvider
Array
(
    [strpos] => 0.077999114990234
    [substr] => 0.13390588760376
    [startswith] => 0.28392887115479
)
As per the discussion in #30952 and according to my tests, `strpos` is considerably faster than calling startsWith for short haystacks.
@36864 36864 changed the title Optimize package registration Optimize Service Provider registration Dec 27, 2019
@xmoroccan

This comment has been minimized.

Copy link

xmoroccan commented Dec 27, 2019

Huge difference according to your benchmarks, thank you!
Nobody says no to performance. I wish this get merged (I haven't reviewed the code)

@GrahamCampbell GrahamCampbell changed the title Optimize Service Provider registration [6.x] Optimize Service Provider registration Dec 27, 2019
@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 27, 2019

Do you actually see a difference, compared to running the entire app though?

@36864

This comment has been minimized.

Copy link
Contributor Author

36864 commented Dec 27, 2019

Well, no. We are clearly in micro-optimization territory here.

But I don't see a reason for the framework to use Str::startsWith when it is known that the needle is not an array, and the length of the haystack is also probably not much greater than 100 characters.

Further tests showed substr_compare to be even faster in "real use case" benchmarks but slower in more generic benchmarks. Further testing would be needed.

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 27, 2019

I just feel this kind of micro-optimization is too far. The overhead of creating the collection and using the parturition method is probably more than the speedup you get from this change.

@dmitryuk

This comment has been minimized.

Copy link
Contributor

dmitryuk commented Dec 28, 2019

Cheaters )

@taylorotwell taylorotwell merged commit befe49c into laravel:6.x Dec 28, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@36864 36864 deleted the 36864:patch-1 branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.