Performance optimization #64
Conversation
|
Strange. I don't know my master's tests pass, because composer missing |
| @@ -15,7 +15,8 @@ | |||
| }, | |||
| "require": { | |||
| "php": "^5.5 || ^7.0", | |||
| "container-interop/container-interop": "~1.0" | |||
| "container-interop/container-interop": "~1.0", | |||
| "zendframework/zend-stdlib": "^2.4" | |||
There was a problem hiding this comment.
If it used for make the test run then should be added to require-dev
There was a problem hiding this comment.
There was a problem hiding this comment.
That class looks like a good candidate for optimization and for dropping a
dependency ;-)
On Jan 13, 2016 00:58, "Witold Wasiczko" notifications@github.com wrote:
In composer.json
#64 (comment)
:@@ -15,7 +15,8 @@
},
"require": {
"php": "^5.5 || ^7.0",
"container-interop/container-interop": "~1.0""container-interop/container-interop": "~1.0","zendframework/zend-stdlib": "^2.4"It's used here
https://github.com/zendframework/zend-servicemanager/blob/master/src/Config.php#L62—
Reply to this email directly or view it on GitHub
https://github.com/zendframework/zend-servicemanager/pull/64/files#r49533959
.
|
#68 should fix tests |
| @@ -164,7 +171,6 @@ public function getServiceLocator() | |||
| public function get($name) | |||
| { | |||
| $requestedName = $name; | |||
| $name = isset($this->resolvedAliases[$name]) ? $this->resolvedAliases[$name] : $name; | |||
There was a problem hiding this comment.
I'm not sure this will work. If you have an alias set, asking the alias the first time, then the resolved name the second time will create the instance two times instead of one.
There was a problem hiding this comment.
@bakura10 this line is just move after next if statement. We don't need $name before this check
|
I have rebased to current stable master and all tests pass! |
|
Looks good to me now :). Thanks for the improvement! |
| @@ -172,6 +178,8 @@ public function get($name) | |||
| return $this->services[$requestedName]; | |||
| } | |||
|
|
|||
| $name = isset($this->resolvedAliases[$name]) ? $this->resolvedAliases[$name] : $name; | |||
There was a problem hiding this comment.
Dang, wish we had PHP7's ?? for this.
|
I just noticed that the entire benchmark setup is completely broken. Not good. |
|
Yep. See that: #62 |
|
@bakura10 no, it's not that. See zend-servicemanager/benchmarks/FetchServices.php Lines 38 to 43 in 218eb28 How many of those are actually new services? How many were already instantiated? |
|
Haaaa. You're right!! Didn't spot that. Good catch. |
|
I'm currently migrating to phpbench to have some actual usable benchmark outputs (mainly because the output is too different between runs). |
|
You have absolutely right. I had plan to fix that soon |
|
@snapshotpl hold yer horses: I'm sending a patch against your branch in a few. |
|
@snapshotpl see snapshotpl#1 - feel free to review/merge, it will be included in this PR afterwards. |
…al-aliases-performance-optimization Feature - zendframework#64 additional aliases performance optimization
|
@Ocramius looks great. Nice performance boost coming in next patch ;) |
|
Great to see we still managed to squeeze a bit of performance (I honestly didn't think it was possible :-). Feel free to come with a 20% performance improvement to Zend Expressive too ^^. |
|
@bakura10 I see a lot of space for micro-optimizations with great impact in here, although it really needs phpbench/phpbench#252 before we even think of making the code harder to read :-P |
…rformance-optimization Feature - #64 additional aliases performance optimization
|
@snapshotpl merged, thanks! |
I have deeply reviewed code of v3 and I have found some interesting performance improvements.