Make WordPress Core

Opened 5 weeks ago

Closed 5 weeks ago

Last modified 5 weeks ago

#64079 closed task (blessed) (fixed)

Update Sodium Compat to 1.23.0 for better performance

Reported by: paragoninitiativeenterprises's profile paragoninitiativeenterprises Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.9 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: needs-patch
Focuses: performance Cc:

Description

Hi WordPress team. I've been busy since #64008 working to improve performance and test coverage

This change affects a lot of internal classes, but see https://github.com/paragonie/sodium_compat/pull/198 for what this much diff buys you. That's about a 7% to 12% increase for the PHPUnit test suite across PHP versions. The more you touch curve25519, without ext-sodium installed, the more pronounced this performance benefit will be.

Change History (6)

#1 @paragoninitiativeenterprises
5 weeks ago

Sorry, forgot to highlight something from v1.22.0:

The previous version of sodium_compat was overly permissible with sodium_base642bin() when the *_NO_PADDING variants were specified, which was not compatible with ext-sodium. This has been fixed in v1.22.0.

I doubt many WordPress sites or plugins used this feature to begin with, but I wanted to make sure it was flagged.

#2 @johnbillion
5 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.9

Thanks for the ticket @paragoninitiativeenterprises 👍

#3 @johnbillion
5 weeks ago

  • Type changed from defect (bug) to task (blessed)

#4 @SergeyBiryukov
5 weeks ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 60905:

Upgrade/Install: Update sodium_compat to v1.23.0.

The previous version of sodium_compat was overly permissible with sodium_base642bin() when the *_NO_PADDING variants were specified, which was not compatible with ext-sodium. This has been fixed in version 1.22.0.

Version 1.23.0 includes some optimizations by replacing the array in the Curve25519 field element with 10 integer object properties instead. The result is a 7% to 12% speedup for the overall PHPUnit suite.

References:

Follow-up to [55699], [58752], [58753], [60787].

Props paragoninitiativeenterprises, SergeyBiryukov.
Fixes #64079.

#5 @johnbillion
5 weeks ago

@SergeyBiryukov This sort of change really needs to go in via a PR first instead of being committed directly without even a patch file. The tests didn't have a chance to run prior to commit. Nowhere is it documented how the changes in this commit were generated.

#6 @paragoninitiativeenterprises
5 weeks ago

@johnbillion You raise a good point. Happy to attest to the changeset this time:

Scrolling through https://core.trac.wordpress.org/changeset/60905, it appears to be the diff from https://github.com/paragonie/sodium_compat/compare/v1.21.2...v1.23.0 without the changes to our tests/ directory. I don't observe any errant additions or exclusions beyond the omission of tests (which is normal for a sodium_compat update).

If anything does break from this update, it would be astonishing to me and should be investigated as a sodium_compat bug rather than a patch import issue.

Note: See TracTickets for help on using tickets.