Improve performance of bcrypt related unit tests

[desrosj]

It seems that when using PHP 8.4, there is a considerable decrease in overall performance in combination with MySQL databases.

This was spotted because there have been more frequent occurrences of the PHPUnit test suite failing due to the configured 20 minute timeout being exceeded. For an example, see the annotations at the ​bottom of this run summary. The related jobs take 2-3x longer to complete.

The Performance Testing workflow runs using the latest tag for PHP, which currently is applied to the 8.2 container where this decrease is not apparent.

[desrosj]

Some additional context. There was a ​brief conversation in Slack around this where build tool specific issues were mostly ruled out.

I've also gone and created a PR that runs the performance tests using PHP 8.4, and it seems to confirm that there is a ​1-5% decrease in nearly all metrics across the board. I also tried using MySQL 9.2 and the results were more mixed (he majority of metrics declined, though not by as much).

For the short term, I am going to change the timeout value for the PHPUnit workflows when 8.4 is being tested to prevent cancellations. The conservative timeout value did it's job here and helped us find a performance problem! 🙌

I'm also going to milestone this for 6.8 for investigation purposes. If there's nothing actionable prior to RC1, we can punt and continue exploring for 6.9.

[desrosj]

In 59873:

Build/Test Tools: Raise timeout value when running tests on PHP 8.4.

The test suite when run on PHP 8.4 with MySQL is currently taking 2-3x the amount of time to run. The jobs are regularly hitting the conservative 20 minute time out configured to prevent runaway jobs.

While this performance regression is investigated, this increases the timeout value to 30 to avoid running into unnecessary failures now that the issue has been discovered.

Props johnbillion.
See #63026.

The PHPUnit test suite when run on PHP 8.4 with MySQL 8.4 seems to be taking 2-3x the amount of time as all other jobs. This switches the performance testing workflow to use PHP 8.4 to see if there is a noticeable change in measured performance.

​See Slack conversation.

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

[siliconforks]

Perhaps the new password hashing (#21022) might be a factor here? It seems that password_hash() with BCrypt is (intentionally) slower in PHP 8.4 than in earlier versions.

​https://wiki.php.net/rfc/bcrypt_cost_2023

[johnbillion]

Ok the culprit is indeed the commit that introduced the switch to bcrypt. The run time for the PHP 8.4 unit test jobs on GitHub Actions jumped from around 7-8 minutes to around 14-15.

Some notes:

• The unit tests for all PHP versions, not just 8.4, have slowed. The impact on 8.4 is the greatest, presumably due to the higher bcrypt cost.
• Prior to the commit I had run several rounds of before and after performance tests of the test suite locally, however I was only doing this with the --group auth filter. The difference is less noticeable when only testing a subset, and there was an expected increase in the test time anyway due to an increase in the number of tests and the fact they make heavy use of the password hashing and checking functions.
• The root cause is likely due to the high number of tests that make use of user fixtures. Each time a user fixture is created, a hash of their password is generated, which was previously handled by phpass but is now handled buy bcrypt which, by design, is slower.
• Ideally any tests that need a user fixture but don't care about the user's password would bypass the password hashing, but that will require adjustments to tests relating to authentication.

[johnbillion]

There are a over 1,240 calls to create a user via the user factory when the tests run (single site standard test suite). Each of these calls causes a password hash to be generated.

[johnbillion]

Removing the performance focus as this is confined to the tests and the performance tests are not otherwise indicating a regression.

[audrasjb]

As per today's bug scrub: adding tests focus and keeping it in the milestone for now.

[desrosj]

Updating the title to reflect that this is more pronounced in but not specific to 8.4.

Closing out. This was determined to be caused by the new bcrypt tests.