Make WordPress Core

Opened 6 months ago

Closed 2 weeks ago

#60127 closed task (blessed) (fixed)

Fix the Performance Testing workflow in the 6.4 branch

Reported by: joemcgill's profile joemcgill Owned by: joemcgill's profile joemcgill
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.5
Component: Build/Test Tools Keywords: has-patch
Focuses: performance Cc:

Description

Since [57198] all runs of the Performance Workflow on the 6.4 branch are failing on the "Run baseline performance tests" step.

This is likely due to that step trying to run performance tests against BASE_TAG version that has been set to 6.1.1 since the workflow was introduced, since that WP version did not support PHP 8.3.

Some potential options to consider:

  1. Update the version used for the baseline tests each major version.
  2. Set the PHP version used by performance tests to the current recommended version, rather than the latest version.
  3. Run benchmarks using independent environments rather than changing the WP version on the same install between tests.

Change History (34)

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


6 months ago

#2 @joemcgill
5 months ago

  • Focuses performance added

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

#6 @swissspidy
5 months ago

  • Keywords needs-patch added

Update the version used for the baseline tests each major version.

This probably makes the most sense. Gutenberg updates the baseline version regularly as well.

Set the PHP version used by performance tests to the current recommended version, rather than the latest version.

That sounds reasonable to me.

Run benchmarks using independent environments rather than changing the WP version on the same install between tests.

That probably negates the whole point of the baseline comparison as the tests are no longer running on the same environment.

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


4 months ago

#14 @chaion07
4 months ago

Thanks @joemcgill for reporting this. We reviewed this ticket during a recent bug-scrub session. Here's the summary of the feedback received:

  1. Still needs a patch
  2. @thelovekesh is willing to work on it as per this discussion
  3. @thelovekesh mentioned that he is looking in to it and would take this up following some discussion with the core performance team.

Cheers!

Props to @thelovekesh @poena @kirasong and @adarshposimyth for the discussion

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 months ago

#16 @mukesh27
4 months ago

  • Owner set to thelovekesh
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


3 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 months ago

#19 @swissspidy
3 months ago

  • Milestone changed from 6.5 to 6.6

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 weeks ago

#21 @desrosj
5 weeks ago

While discussing the performance testing workflow in the context of #61213 with @swissspidy today, we questioned whether running these tests in older branches has any benefits.

Instead of fixing this, would it make more sense to only run the workflow on branches that support the same versions of PHP? Or only run on trunk and the currently supported major version branch?

#22 @joemcgill
4 weeks ago

Or only run on trunk and the currently supported major version branch?

I think this probably makes sense, or at minimum, only trigger error notices on those two branches.

At some point, we've switched trunk back to running these on PHP 8.2, but that didn't get backported the 6.4 branch. That would also fix this issue.

Somewhat related, we've been using the 6.1.1 release as the base branch for long enough that we're starting to see some strange drift show up in our dashboard metrics (related discussion). Bumping the base branch each release, like the Gutenberg performance tests do, could also help keep these running longer term.

#23 @joemcgill
4 weeks ago

Actually, looking into the PHP version issue more, I didn't realize that the problem was that you were pinning old branches only to the highest supported PHP version in #60095. You've not done the same for the 6.5 branch yet, but will end up having the same effect on the Performance workflow when that occurs.

I'm thinking that bumping the BASE_TAG value is probably worth trying regardless, but we could also fix the 6.4 branch by defining LOCAL_PHP=8.2-fpm in the workflow.

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


4 weeks ago
#24

  • Keywords has-patch added; needs-patch removed

@joemcgill commented on PR #6587:


4 weeks ago
#25

The Performance Tests are passing with PHP pinned to 8.2.

#26 @joemcgill
4 weeks ago

  • Owner changed from thelovekesh to joemcgill

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 weeks ago

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


3 weeks ago
#28

This adds a php-version input to the reusable performance testing workflow. This is to accommodate the changes to fix the workflow in the 6.4 branch once that branch is updated to make use of the new reusable workflows.

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

#29 @desrosj
3 weeks ago

Thanks all! It does indeed look like the PR fixes the issue.

I've opened an additional PR that updates the reusable performance test workflow in trunk to accept a PHP version as well. This will ensure that a specific PHP version can be configured when the changes associated with #61213 through [58165] and [58166] are backported to the 6.4 branch.

@joemcgill commented on PR #6587:


2 weeks ago
#30

@desrosj, given the state of #6674 do you think we should commit this or wait and update the workflow in the 6.4 branch to use the new reusable workflow?

@desrosj commented on PR #6587:


2 weeks ago
#31

Because this is a unique fix that has not yet been committed to trunk/a later branch, let's merge this separately and then follow up with the changes in #6674, and then #6385.

#32 @desrosj
2 weeks ago

In 58269:

Build/Test Tools: Add input for PHP version to the performance workflow.

This adds a new input to the reusable performance testing workflow for accepting a PHP version. This allows the workflow to be reused in older branches when the PHP version currently tagged latest was not supported.

Props swissspidy, joemcgill.
See #60127, #61213.

#34 @desrosj
2 weeks ago

In 58270:

Build/Test Tools: Correct conditional logic in [58269].

See #60127, #61213.

#35 @joemcgill
2 weeks ago

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

In 58272:

Build/Test Tools: Fix the Performance workflow in the 6.4 branch.

This pins the LOCAL_PHP version to 8.2 in the Performance workflow to ensure that the base measurements taken from WordPress version 6.1.1 can be run, since PHP 8.2 is the greatest version that WordPress 6.1.1 supports.

The default PHP version used in the 6.4 branch was bumped to 8.3 in [57198] to reflect the highest version that WordPress 6.4 supports, resulting in the failures to the Performance workflow.

Props joemcgill, freewebmentor, desrosj, swissspidy.
Fixes #60127.

Note: See TracTickets for help on using tickets.