Make WordPress Core

Opened 5 weeks ago

Closed 10 days ago

#65861 closed enhancement (fixed)

Backport the bcrypt unit test performance fixes to the 6.8 branch

Reported by: lucatume Owned by: lancewillett
Priority: normal Milestone: 7.2
Component: Build/Test Tools Version: 6.8
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

6.8 branched on 2025-03-25, before [60253] and [60298] landed on trunk under #63026.
It is the only branch that ships bcrypt password hashing without the accompanying test
suite cost reductions: 6.7 and earlier predate bcrypt, 6.9 and later inherited both
changesets.

Every PHP version pays for it. Median job duration, run 31596990187 on 6.8 against run
31600298705 on 6.9, an hour apart on matrices of the same size:

PHP 6.8 6.9
7.2 818s 754s
7.4 807s 768s
8.0 825s 752s
8.3 855s 790s

PHP 8.4 is worse again, because it raised the default bcrypt cost from 10 to 12 while
[60298] pins the test cost to 5. On PHP 8.4 / MySQL 8.4 example.org:8889, the one job
name common to both branches, the Run PHPUnit tests step takes 999s on 6.8 and 701s
on 6.9.

That tail is what breaks the run. Three jobs exceeded timeout-minutes and were
cancelled, which dispatched failed-workflow.yml and reran the run, for 51m18s of wall
clock against 18m32s for 6.9. p90 was 1134s against 829s.

This matters during a security release, when several branches build at once and 6.8 is
the slowest of them.

Proposed: merge [60253] and [60298] to the 6.8 branch so its suite matches 6.9's. Both
apply without conflict.

Change History (3)

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


5 weeks ago
#1

  • Keywords has-unit-tests added

6.8 branched on 2025-03-25, before 60253 and [60298] landed on trunk under #63026, so it's the only branch that ships bcrypt password hashing without the accompanying test suite cost reductions. 6.7 and earlier predate bcrypt; 6.9 and later inherited both changesets.

Every PHP version pays for it. Median job duration, run 31596990187 on 6.8 against run 31600298705 on 6.9, an hour apart on matrices of the same size:

PHP 6.8 6.9
7.2 818s 754s
7.4 807s 768s
8.0 825s 752s
8.3 855s 790s

PHP 8.4 is worse again, because it raised the default bcrypt cost from 10 to 12 while [60298] pins the test cost to 5. On PHP 8.4 / MySQL 8.4 example.org:8889, the one job name common to both branches, the Run PHPUnit tests step takes 999s on 6.8 and 701s on 6.9. That job is left out of the table above because 6.9 runs PHP 8.4 only on the two example.org:8889 variants, so a median would compare 26 jobs against 2.

That tail is what breaks the run. Three jobs exceeded timeout-minutes and were cancelled, which dispatched failed-workflow.yml and reran the run, for 51m18s of wall clock against 18m32s for 6.9. p90 was 1134s against 829s.

This merges both trunk changes so 6.8's suite matches 6.9's:

  • [60253], developed in #8657, moves per-test user creation into shared fixtures. Users created in wpSetUpBeforeClass hash at PHP's full default cost, since the cost filter is registered in set_up().
  • [60298], developed in #8453, pins the bcrypt cost to 5 for the hashes that filter does cover.

Both applied without conflict.

Two things for review:

reduce_hash_cost() now produces cost 4, PHP's minimum, which is why the pinned cost is 5 and not 4.

[60253] touches 28 test files that have had 16 months to drift on this branch. The merge was clean, but that only means the text lined up. The CI run on this PR is the real check, and breakage would most likely show as order dependence in the customize and meta suites.

#2 @lancewillett
11 days ago

  • Milestone Awaiting Review7.2
  • Owner set to lancewillett
  • Status newaccepted

#3 @lancewillett
10 days ago

  • Resolutionfixed
  • Status acceptedclosed

In 63439:

Build/Test Tools: Improve bcrypt-related unit test performance.

Merges [60253] and [60298] to the 6.8 branch.

Move repeated user creation into shared fixtures and lower the bcrypt cost during tests. This reduces suite runtime and prevents CI timeouts, especially on PHP 8.4.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13018

Reviewed by: desrosj.
Props peterwilsoncc, johnbillion, lucatume, adrianmoldovanwp.
Fixes #65861.
See #63026.

Note: See TracTickets for help on using tickets.