[11.x] Optimize boostrap time by using hashtable to store providers

[sarven]

It should slightly optimize bootstrap time, especially for large applications with many providers. I've encountered that the method getProvider has O(n) complexity, but it could be optimized to O(1).

• Application::register
• Application::getProvider

Of course, this time from Blackfire is not true, and the original value is smaller, but still executing that function ~63k times isn't a good thing.

Seems like serviceProviders hashtable is enough, and we can get rid of loadedProviders.

[browner12]

Maybe I'm not understanding something here, but isn't the point of having separate "registered" and "loaded" service providers so we can defer loading of ones that we won't necessarily need on every request? Does this change maintain that functionality?

[sarven]

@browner12 if you mean deferred providers, those changes don't touch that functionality directly. Deferred providers are in the property: deferredServices

loadProviders property was used only in the method: markAsRegistered, where a provider is added to serviceProviders, so it looks like using only one array as the source of truth should be ok.

[taylorotwell]

Can you share benchmarks of speed improvements on a blank Laravel application that makes a single database query?

[bert-w]

Hasn't logic changed now? Because getProvider() used to check for instanceof (in getProviders()) and now it doesnt anymore.

[sarven]

@taylorotwell A fresh application contains a small number of providers, so it isn't even shown by Blackfire as a significant cost. However, I've added 300 providers to some fresh app, and here are the results:

BEFORE

AFTER

After my change, Blackfire doesn't show that method, because the cost is insignificant. There are ((n - 1) * n) / 2 of executions, so for small apps, it isn't important, but for bigger applications with more providers like 100, 200, 300, or even more it should be a nice optimization having an impact on every request.

@bert-w it shouldn't be a problem for getProvider method, as there is always returned only one. The method getProviders returns results in the same way comparing by instanceof, which is used in some places to get e.g. all events providers by the base class. Or maybe, I overlooked something?

[driesvints]

Remember that Taylor doesn't sees draft PR's. Please mark this as ready if you need another review.

[sarven]

@driesvints thanks for clarifying

@taylorotwell could you have another look? I'm not sure if you received a notification about my response, because it was written when the PR was still marked as a draft.

[taylorotwell]

Sorry, maybe I'm missing it, but how long does it take to run an entire Laravel request (in milliseconds) for a normal app with a single database query before and after this PR?

[sarven]

@taylorotwell As I've written, this isn't relevant for small apps with only a few providers. So I can't prepare such a comparison for "a normal app with one database query", because it isn't possible to show any optimization in that case. It would be useful only for larger apps with many providers. I've seen some monolithic apps using Laravel with over 300 providers, so this isn't unusual. Here is a comparison of an app with 300 providers:

ab -n 1000 -c 1 http://127.0.0.1:8080/

BEFORE

Server Software:        nginx/1.25.5
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /
Document Length:        20 bytes

Concurrency Level:      1
Time taken for tests:   24.429 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      1146000 bytes
HTML transferred:       20000 bytes
Requests per second:    40.93 [#/sec] (mean)
Time per request:       24.429 [ms] (mean)
Time per request:       24.429 [ms] (mean, across all concurrent requests)
Transfer rate:          45.81 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.5      0       5
Processing:    17   24   8.3     22      95
Waiting:       17   24   8.3     22      95
Total:         17   24   8.3     22      95

Percentage of the requests served within a certain time (ms)
  50%     22
  66%     24
  75%     24
  80%     25
  90%     27
  95%     32
  98%     65
  99%     70
 100%     95 (longest request)

AFTER

Concurrency Level:      1
Time taken for tests:   20.777 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      1146000 bytes
HTML transferred:       20000 bytes
Requests per second:    48.13 [#/sec] (mean)
Time per request:       20.777 [ms] (mean)
Time per request:       20.777 [ms] (mean, across all concurrent requests)
Transfer rate:          53.86 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.2      0       4
Processing:    14   20   6.7     19      83
Waiting:       14   20   6.7     18      82
Total:         15   21   6.7     19      83

Percentage of the requests served within a certain time (ms)
  50%     19
  66%     20
  75%     21
  80%     22
  90%     24
  95%     32
  98%     44
  99%     59
 100%     83 (longest request)

BTW It is hard to measure fairly, because the difference is quite small 2-3ms per request, so the best way to present that optimization is to look at the profiler (screens in my previous comment).

[taylorotwell]

@sarven sorry to drag this PR out a bit, but is it at all possible to make the relevant performance improvements with minimal others changes? For example while keeping the loadedProviders property?

[sarven]

@taylorotwell no problem, changed. I thought also about creating a new private method like getProviderByClassName to keep the behavior of the current getProvider the same. Currently, for example, it's possible to get the first provider of the base class like ServiceProvider, because there is used instanceof, but it doesn't seem like something useful and I assume that no one uses that method in that way, so changing the behavior a little bit shouldn't be a problem. What do you think?