Use isset() instead of array_key_exists() in DIC #8907
Conversation
|
This is wrong as some services can be |
|
Yes I'm sure about xdebug being disabled (although it looks like you deleted your comment?). isset() || array_key_exists() handles the case of a null service, and still gives me significant reduction in calls. |
|
didn't we go back and forth here before? |
|
Well remembered. I did a search and found it discussed here: #7007 (diff) But isset() || array_key_exists() wasn't mentioned in that exchange. |
|
@catch56 Does it bring a noticeable performance improvement on a full page? |
|
It's 0.8 of a millisecond on my local install and 1/45th of the total function calls on the page. That's not huge but yes it's a measurable improvement (i.e. ten similar changes and they'd add up to 8ms). |
|
I think we can likely save some of the 1,200 or so strtolower() calls here as well - by checking for the string as passed in first in the common cases, then falling back to strtolower() later. That's roughly a millisecond on my machine as well - will put it in a new pull request but it'll need to depend on this one since it's the same lines of code. |
|
@catch56 As this is also a performance optimization, feel free to do everything in this PR. That makes things easier for everyone. Thanks. |
|
OK sounds good, thanks! |
|
Pushed that commit. Here's before after xhprof UI screenshots showing the calls removed.
|
|
Because that's Drupal coding standards which my brain defaults to. Updated. |
This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #8907). Discussion ---------- Use isset() instead of array_key_exists() in DIC It doesn't look like the array_key_exists() in Container::get() and Container::has() is necessary. Changing this to isset() removed over 1,000 calls on a default Drupal 8 install - attaching before/after xhprof screenshots with this change.   Commits ------- 3c01ae6 Use isset() instead of array_key_exists() in DIC
|
merged in 2.3, thanks. |
| // available services. Service IDs are case insensitive, however since | ||
| // this method can be called thousands of times during a request, avoid | ||
| // calling strotolower() unless necessary. | ||
| foreach (array(false, true) as $strtolower) { |
There was a problem hiding this comment.
Does flatten code would be more efficient than this loop ?
It doesn't look like the array_key_exists() in Container::get() and Container::has() is necessary. Changing this to isset() removed over 1,000 calls on a default Drupal 8 install - attaching before/after xhprof screenshots with this change.