Make WordPress Core

Opened 13 months ago

Closed 4 months ago

Last modified 2 months ago

#57067 closed enhancement (fixed)

Replace certain `array_key_exists()` calls with `isset()` once PHP requirement is raised to 7.0+

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch commit
Focuses: performance Cc:

Description

This ticket is blocked until WordPress requires at least PHP 7.0 as minimum version.

Checks like isset( MY_CONSTANT[ $key ] ) result in a fatal error in PHP < 7.0, which is why parts of the WordPress codebase uses array_key_exists() for such situations as of today, due to the support for PHP 5.6. Also see https://github.com/PHPCompatibility/PHPCompatibility/issues/1301 which explores flagging this as part of the PHPCompatibility sniffs.

This was raised as part of #56974, which reviewed several of these checks. isset is slightly preferable over array_key_exists in terms of performance (with the caveat that the two behave differently when it comes to null values).

As of today, this ticket is essentially a reminder to update such calls to array_key_exists() in WordPress core once it will at some point no longer support PHP versions below 7.0.

Change History (19)

#1 @flixos90
13 months ago

cc @jrf @aristath @desrosj

#2 @flixos90
13 months ago

In 54804:

Editor: Improve performance of WP_Theme_JSON class by reducing usage of expensive array functions.

In many scenarios array functions are more expensive than using simpler for or foreach loops.

This changeset results in roughly 4% faster wp_head execution time for both block themes and classic themes. While this may seem like a small win, it is a worthwhile enhancement and only one part of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props aristath, desrosj, jrf, spacedmonkey.
Fixes #56974.
See #57067.

#3 @flixos90
13 months ago

In 54805:

Editor: Improve performance of WP_Theme_JSON class by reducing usage of expensive array functions.

In many scenarios array functions are more expensive than using simpler for or foreach loops.

This changeset results in roughly 4% faster wp_head execution time for both block themes and classic themes. While this may seem like a small win, it is a worthwhile enhancement and only one part of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props aristath, desrosj, jrf, spacedmonkey.
Merges [54804] to the 6.1 branch.
Fixes #56974.
See #57067.

This ticket was mentioned in PR #3907 on WordPress/wordpress-develop by @Mamaduka.


11 months ago
#4

  • Keywords has-patch added

PR fixes a small regression after 54805, where an incorrect variable was used inside the loop.

Trac ticket: https://core.trac.wordpress.org/ticket/57067

#5 @flixos90
10 months ago

In 55142:

Editor: Fix undefined variable following [54805].

Props mamaduka, costdev, mukesh27.
See #56974, #57067.

#7 @johnbillion
7 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Future Release to 6.3

#8 @joemcgill
6 months ago

The effort to bump the minimum version of PHP is being tracked in #57345, which is blocking this ticket. Even so, it would be good to have a patch ready and reviewed so it can be committed as soon as the official decision to drop PHP 5.6 has been made.

Last edited 6 months ago by joemcgill (previous) (diff)

#9 @flixos90
6 months ago

Anybody willing to be the owner on this ticket and work on a pull request? Otherwise, I think we'll have to punt it, since the final decision on whether to bump the minimum PHP version has still not been made.

We need a PR for this preferably by end of this week in order to commit it before 6.3 beta next Tuesday.

#10 @johnbillion
6 months ago

  • Milestone changed from 6.3 to Future Release

Let's bump it for now, it's not a critical change. I'm waiting to hear the final decision about the minimum PHP version bump.

#11 @GaryJ
5 months ago

Since the bump to PHP 7.0 happened for WP 6.3, can this be brought back into 6.3 as well?

#12 @johnbillion
5 months ago

  • Milestone changed from Future Release to 6.4

It's a minor change however it's not urgent and it's an enhancement. If someone works on a PR we'll get it into 6.4.

#13 @Soean
4 months ago

I created a PR for the WP_Theme_JSON in the Gutenberg project: https://github.com/WordPress/gutenberg/pull/53098

After merging, I will create a PR for Core.

#15 @flixos90
4 months ago

  • Keywords has-patch commit added; needs-patch removed
  • Owner set to flixos90
  • Status changed from new to reviewing

Thanks @Soean for the PR, about to commit shortly.

#16 @flixos90
4 months ago

  • Component changed from General to Themes

Changing the component since this is exclusive to the WP_Theme_JSON class.

#17 @flixos90
4 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 56345:

Themes: Use isset instead of array_key_exists in WP_Theme_JSON class.

With the minimum PHP version requirement raised to 7.0, we can now use isset on constants that are arrays. Since isset is slightly faster than array_key_exists (and the different handling of null values is irrelevant for the updates here), remaining instances of array_key_exists in the WP_Theme_JSON class are replaced in this changeset.

Props soean.
Fixes #57067.

#18 @spacedmonkey
2 months ago

  • Keywords needs-dev-note added

#19 @spacedmonkey
2 months ago

  • Keywords needs-dev-note removed
Note: See TracTickets for help on using tickets.