Ocramius
commented
6 years ago The number of revolutions is not set to a default (currently 1.000) which would make sure, that the numbers computed are reliable enough to derive cases for action upon them. Did anybody run the benchmarks with that intention in mind? To identify the need for optimization? What did you find out?
I usually run with higher revs locally. I think the defaults were set to not bother CI, just to prevent the benchmark suite to rot (it should keep running)
Several use cases are not benchmarked at all. There is not a single test for ::has (this will change. my PR #217 regarding that was just accepted :)).
The suite was added as part of the 3.0 work, and only covers hot paths. has is usually not hot-path.
There is no benchmark in place for delegators right now. @Ocramius, you know, you are one of my most important teachers. That said, where are the delegator tests? I don't think it would be ok to let them out.
While interesting to test, delegators are likely the 1% scenario out there, not hot path. Also, they are quite heavy anyway, as they add:
- A runtime closure declaration
- Multiple recursive calls to an instantiation (one per delegator)
Also, multiple scenarios are to be tested with delegators, so the effort was not worth it compared to first getting to a working test suite. Feel free to add new PRs with scenarios if you have time for it though 👍
Furthermore, service creation via deprecated approaches is measured (invokables, for example), but their replacement through currently active and recommended interfaces (InvokableFactory, for example) is not measured. Who did that and what was the reason not to go all the way down to the benchmarks?
This is worth inspecting. Possibly forgotten at review when invokable was deprecated.
It was a slight shock, when I found, that SetAlias is the second most expensive API, the service manager offers (among those APIs tested at all, the only API more expensive is creating a ServiceManager instance with 1.000 services, factories, (deprecated) invokables and aliases each. Not a realistic use case, but good to know.). IMHO, it's not acceptable that SetAlias can consume up to 35 milliseconds (on my machine only, of course, up to six times more time than any other API, generally spoken). I can't comprehend. Optimizations for that code are quite obvious. Permission and appreciation assumed, I'll take care of that next.
Not sure why so many service definitions are added, but the benchmarks are not designed to compare across each other, but rather against themselves over different library versions, so it is not important if some are heavier than others as long as they have deterministic run-time.
And yep, using bleeding edge PHP is pretty much OK. Upgrading PHP beats any kind of micro-optimisation we can do. Make sure you try it with OpCache too BTW.
I inspected the benchmarks. Here's what I found
Because all ServiceManager APIs are quite fast, the revolutions setting (revs) should be bigger. A setting of 100.000 seems to be ok for me. The only exception is FetchNewServiceManagerBench.php. 100.000 service managers blow up even the best machine. Here I recommend 500 revolutions per default.
The number of iterations can savely be increased to 100 for all benchmarks.
There is no benchmark in place for delegators right now.
Service creation via deprecated approaches is measured (invokables, for example), but their measurement for their replacements through currently active and recommended interfaces (InvokableFactory, for example) are not in place..
SetAlias is the second most expensive API, the service manager offers. IMO this should be optimized. Permission assumed, I'll take care of that next.
SetNewServicesBench.php is incomplete. It does not cover all options to create a new service.
Summary:
Running the benchmarks with the current defaults may lead to disappointment. They numbers obtained may be less expressive than expected. Several features are not benchmarked yet. Some optimization options were not taken. SetAlias is more expensive than necessary.
Attached is a zip documenting the issues found. The .txt files show the command line with phpbench output (--progress=blinken), the xml files document the outcome. All tests were run with revs=100.000 and iterations=100, except FetchNewServiceManager, which was run with revs=500, iterations=100.
benchmarks.zip
Ah, please don't blame me for using a (more or less) current dev build of PHP 7.3. :)