D7 ThemeRegistry array_key_exists() micro-optimization

This is trivial yet effective. Moving to RTBC.

Looks nice to me, but let's get it into Drupal 8 first.

I missed that there is already a parent issue for Drupal 8 (#2770065: array_key_exists() micro-optimization) - although it looks like maybe it only contains one of the fixes from this issue, but not both?

Postponing this on that issue for now.

+++ b/includes/theme.inc
@@ -418,7 +418,7 @@ class ThemeRegistry Extends DrupalCacheArray {
@@ -428,7 +428,7 @@ class ThemeRegistry Extends DrupalCacheArray {

@@ -428,7 +428,7 @@ class ThemeRegistry Extends DrupalCacheArray {
     if (isset($this->storage[$offset])) {
       return $this->storage[$offset];
     }
-    elseif (array_key_exists($offset, $this->storage)) {
+    elseif (isset($this->storage[$offset]) || array_key_exists($offset, $this->storage)) {
       return $this->resolveCacheMiss($offset);
     }

Probably this is unnecessary since the previous statement is an isset()

Otherwise looks good to me :)

joelpittet , the D8 issue is stalled right now, it needs a reroll, they want it to be a smaller patch like this one, see the last couple comments. Also, what versions of php are you XHProf 'ing on?

wondering if PHP 7 also sees performance improvement with this patch, and if so, by how much.

We can reopen this issue now that #2770065: array_key_exists() micro-optimization is in. Moving to NW for #8

here is an updated patch and interdiff which implements as per #8

@Dinesh18 thanks for the patch

+++ b/includes/theme.inc
@@ -428,9 +428,7 @@ class ThemeRegistry Extends DrupalCacheArray {
-    elseif (array_key_exists($offset, $this->storage)) {
-      return $this->resolveCacheMiss($offset);
-    }
+    return $this->resolveCacheMiss($offset);
   }

array_key_exists here is needed, this change should be reverted (in #8 I meant only the isset() part :))

Here is an updated patch and interdiff as per comment #14

RTBC #15

I agree, #15 looks good.
(if it helps)

Looks good to me. I am a fan of micro optimizations like this one :).

Pending Commit.

Easy win here!
7.60?

Feel the need for speed!

RTBC maintain
I am a huge fan of optimisations like this going in, this improves speed for everyone!
FabianX already agreed to this change in #2863786-18: D7 ThemeRegistry array_key_exists() micro-optimization
So I think we have a clear consensus that this is ready.
Also, as mentioned in #12, it's already in D8 #2770065: array_key_exists() micro-optimization

We've put in similar optimisations to this already, and this is another good one. I would very much like to see this go into 7.68

Note: To increase our confidence in the test results for the above issue, these two should go in first so we can re-queue php 7.3 and php 5.3 tests.
#3047844: [Regression] Tests fail on PHP 5.3
#3025335: session_id() cannot be changed after session is started

Maintain RTBC
1) Is already in D8 (similar fix)
2) we already put in performance optimisations similar to this.
3) passes tests
4) has been performance profiled to prove gains

LGTM - we'll try to get this into the next release.

RTBC + 1,

Just noting fun fact that for replacing NULL in arrays that only contain arrays an object of stdClass can be used as the NULL placeholder.

The reason is that `instanceof stdClass` is also not needing a function call, so can be cheaply checked.

So then you can get away with only isset() and instanceof operators and no function calls.

Thanks everyone!