Eliminate anti-pattern isset($arr[$key]) || array_key_exists($key, $arr)

This patch omits the instance in drupal_static() which is accounted for already in the patch for 🐛 drupal_static() edge case bug and inconsistent comments Fixed (which inspired this new issue).

+++ b/core/lib/Drupal/Component/DependencyInjection/Container.php
@@ -320,7 +320,7 @@ public function has($id): bool {
   public function getParameter($name): array|bool|string|int|float|NULL {
-    if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters))) {
+    if (!array_key_exists($name, $this->parameters)) {
       if (!$name) {

Should we change this to \array_key_exists() in namespaced code at the same time?

I'm not seeing anything in the Drupal coding standards indicating that function calls in the global name-space should have a backslash prefix. It does say this about global classes, but that's all; and a cursory grep is only showing a tiny number of such function calls anywhere in the codebase.

I don't mind making that change if need be, but it seems out of scope?

I was wrong -- I see now that this tweak is actually required for namespaced code, so I'll go ahead and re-roll the patch.

https://www.php.net/manual/en/migration74.other-changes.php says:

A specialized VM opcode for the array_key_exists() function has been added, which improves performance of this function if it can be statically resolved. In namespaced code, this may require writing \array_key_exists() or explicitly importing the function.

https://bugs.php.net/bug.php?id=79675 indicates use function array_key_exists; is the correct way to explicitly import the function, and following the cues from that bug I can confirm the other approach practically, courtesy of pecl install vld-beta:

$ php -dvld.active=1 -dvld.execute=0 -r 'array_key_exists(1, []);'

line      #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
    1     0  E >   ARRAY_KEY_EXISTS                                 ~0      1, <array>
          1        FREE                                                     ~0
          2      > RETURN                                                   null

$ php -dvld.active=1 -dvld.execute=0 -r 'namespace a; array_key_exists(1, []);'

line      #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
    1     0  E >   INIT_NS_FCALL_BY_NAME                                    'a%5Carray_key_exists'
          1        SEND_VAL_EX                                              1
          2        SEND_VAL_EX                                              <array>
          3        DO_FCALL                                      0
          4      > RETURN                                                   null

$ php -dvld.active=1 -dvld.execute=0 -r 'namespace a; \array_key_exists(1, []);'

line      #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
    1     0  E >   ARRAY_KEY_EXISTS                                 ~0      1, <array>
          1        FREE                                                     ~0
          2      > RETURN                                                   null

#5 is right, we probably should have a coding standard, but until then using the explicit namespace as a performance optimisation is fine - sorry could have been more explicit why we needed to do it but glad you found it, and hadn't come across vld before - looks handy!

I've added the backslash prefix irrespective of namespacing, as I feel that this opcode/no-opcode performance gotcha is ridiculously esoteric, and so it's probably better if the backslash is employed in all cases (there should be a follow-up issue for code not affected by this patch) so that that it'll look wrong to ever see this function called without a backslash.

Opened 🌱 Call PHP native functions fully qualified (like \array_key_exists()) Active .

@jweowu, thanks for this initiative. It's good that PHP fixed this performance bug and will allow us to have code which is simpler. A good initiative for symfony based Drupals that require the very latest PHP.

Applied patch #8

Searched for ") || array_key_exists('" and got 0 results in the repo.

This is good for a readability improvement and now micro-optimization now that PHP has the opcode implementation. We can try to make fully qualified functions a coding standard in 🌱 Call PHP native functions fully qualified (like \array_key_exists()) Active .

Committed/pushed to 10.1.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.