Performance of _wp_normalize_relative_css_links() can be increased >2x

[westonruter]

The _wp_normalize_relative_css_links() function is not currently implemented in an optimal way. It is doing a regex match for all instances of url(...) in a given CSS, and then for each match it checks if it is a relative URL. If so, it then makes the URL absolute and then does another global search & replace in the CSS to replace the relative path with the absolute path. This means for every search there is a secondary search. This secondary str_replace() can be eliminated by replacing the unitial preg_match_all() with a preg_replace_callback(). This makes the function run >2x faster when ​benchmarked being run across each CSS file in Gutenberg and the built-in themes.

$ composer run-script test -- --iterations=10 --revs=500
> phpbench run Bench.php --report=default '--iterations=10' '--revs=500'
PHPBench (1.2.10) running benchmarks... #standwithukraine
with PHP version 7.4.33, xdebug , opcache 

\Bench

    benchTrunk..............................I9 - Mo7.734ms (±2.96%)
    benchPatch..............................I9 - Mo3.527ms (±6.74%)

Subjects: 2, Assertions: 0, Failures: 0, Errors: 0
+------+-----------+------------+-----+------+------------+-------------+--------------+----------------+
| iter | benchmark | subject    | set | revs | mem_peak   | time_avg    | comp_z_value | comp_deviation |
+------+-----------+------------+-----+------+------------+-------------+--------------+----------------+
| 0    | Bench     | benchTrunk |     | 500  | 4,242,960b | 8,066.348μs | +0.76σ       | +2.24%         |
| 1    | Bench     | benchTrunk |     | 500  | 4,242,960b | 8,006.150μs | +0.50σ       | +1.48%         |
| 2    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,815.732μs | -0.32σ       | -0.93%         |
| 3    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,706.406μs | -0.78σ       | -2.32%         |
| 4    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,702.188μs | -0.80σ       | -2.37%         |
| 5    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,649.464μs | -1.03σ       | -3.04%         |
| 6    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,628.240μs | -1.12σ       | -3.31%         |
| 7    | Bench     | benchTrunk |     | 500  | 4,242,960b | 7,785.394μs | -0.45σ       | -1.32%         |
| 8    | Bench     | benchTrunk |     | 500  | 4,242,960b | 8,314.294μs | +1.82σ       | +5.39%         |
| 9    | Bench     | benchTrunk |     | 500  | 4,242,960b | 8,219.126μs | +1.41σ       | +4.18%         |
| 0    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,492.050μs | -0.80σ       | -5.42%         |
| 1    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,440.668μs | -1.01σ       | -6.81%         |
| 2    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,524.826μs | -0.67σ       | -4.53%         |
| 3    | Bench     | benchPatch |     | 500  | 4,242,960b | 4,035.964μs | +1.38σ       | +9.32%         |
| 4    | Bench     | benchPatch |     | 500  | 4,242,960b | 4,178.894μs | +1.96σ       | +13.19%        |
| 5    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,769.186μs | +0.31σ       | +2.09%         |
| 6    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,527.606μs | -0.66σ       | -4.45%         |
| 7    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,900.464μs | +0.84σ       | +5.65%         |
| 8    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,542.204μs | -0.60σ       | -4.06%         |
| 9    | Bench     | benchPatch |     | 500  | 4,242,960b | 3,508.126μs | -0.74σ       | -4.98%         |
+------+-----------+------------+-----+------+------------+-------------+--------------+----------------+

• Replace preg_match_all() and its secondary str_replace() call with preg_replace_callback().
• Fix case where paths beginning with http and https (but not http: and https:) were erroneously not counted as relative.
• Improve code style and readability by consolidating conditions and returning once.

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

[adamsilverstein]

Nice work @westonruter - PR look good to me, thanks for providing the performance benchmarks as well.

[westonruter]

Great. I plan to commit today or tomorrow.

[westonruter]

In 55658:

Script Loader: Optimize performance of _wp_normalize_relative_css_links() by more than 2x.

• Replace preg_match_all() and its secondary str_replace() call with preg_replace_callback().
• Fix case where paths beginning with http and https (but not http: and https:) were erroneously not counted as relative.
• Improve code style and readability by consolidating conditions and returning once.
• Use str_starts_with() consistently instead of strpos().

Follow-up to [52036], [52695], and [52754].

Fixes #58069.
See #54243.

Committed to trunk: https://core.trac.wordpress.org/changeset/55658

[westonruter]

Reopening for 6.2.1 consideration.

[azaozz]

Sorry, a nitpick :)

The commit message says "Improve code style and readability" but I tend to disagree. Both style and readability seem worse after the commit.

Negating a long multi-line list of function calls wrapped in braces doesn't help readability. Better readability can be achieved by a short-circuit imho. I.e. something like (untested):

if (
   str_starts_with( $url, 'http:' ) ||
   str_starts_with( $url, 'https:' ) ||
   str_starts_with( $url, '//' ) ||
   ....
) {
   return $prefix . $url;
}

$absolute_url = ...
// etc.

Using (yet another) style for multi-line conditionals (every || on its own line) doesn't help it either. I know WPCS doesn't flag this as "error" but why should the style here be different than the rest of the WP code?

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

[azaozz]

In [55669]:

Script Loader: Improve code style and readability in _wp_normalize_relative_css_links().

Props: westonruter.
See: 58069.

Fixed in https://core.trac.wordpress.org/changeset/55669.

[audrasjb]

In 55736:

Script Loader: Optimize performance of _wp_normalize_relative_css_links() by more than 2x.

• Replace preg_match_all() and its secondary str_replace() call with preg_replace_callback().
• Fix case where paths beginning with http and https (but not http: and https:) were erroneously not counted as relative.
• Improve code style and readability by consolidating conditions and returning once.
• Use str_starts_with() consistently instead of strpos().

Follow-up to [52036], [52695], and [52754].

Props westonruter, adamsilverstein, azaozz.
Merges [55658] and [55669] to the 6.2 branch.
Fixes #58069.
See #54243.