Script Loader: Guard against exponential recursion during calculation of loading strategy and fetchpriority.

This addresses a performance issue in the recursive `WP_Scripts::get_highest_fetchpriority_with_dependents()` and `WP_Scripts::filter_eligible_strategies()` methods for redundant processing of shared dependencies in complex dependency graphs. To fix this, a `$stored_results` param is introduced which is passed by reference; this variable contains a cache of the calculated results for all scripts handles, so that subsequent calls for the same handle can return the cached value instead of re-computing it.

Developed in #10459

Follow-up to [60704], [60931], [56033].

Props ciobanucatalin, b1ink0, westonruter, mukesh27.
See #61734, #12009.
Fixes #64194.


git-svn-id: https://develop.svn.wordpress.org/trunk@61176 602fd350-edb4-49c9-b593-d223f7449a82

Author: westonruter

src/wp-includes/class-wp-scripts.php

@@ -997,12 +997,17 @@ private function get_eligible_loading_strategy( $handle ) {
 	 *
 	 * @since 6.3.0
 	 *
-	 * @param string              $handle              The script handle.
-	 * @param string[]|null       $eligible_strategies Optional. The list of strategies to filter. Default null.
-	 * @param array<string, true> $checked             Optional. An array of already checked script handles, used to avoid recursive loops.
+	 * @param string                  $handle              The script handle.
+	 * @param string[]|null           $eligible_strategies Optional. The list of strategies to filter. Default null.
+	 * @param array<string, true>     $checked             Optional. An array of already checked script handles, used to avoid recursive loops.
+	 * @param array<string, string[]> $stored_results      Optional. An array of already computed eligible loading strategies by handle, used to increase performance in large dependency lists.
 	 * @return string[] A list of eligible loading strategies that could be used.
 	 */
-	private function filter_eligible_strategies( $handle, $eligible_strategies = null, $checked = array() ) {
+	private function filter_eligible_strategies( $handle, $eligible_strategies = null, $checked = array(), array &$stored_results = array() ) {
+		if ( isset( $stored_results[ $handle ] ) ) {
+			return $stored_results[ $handle ];
+		}
+
 		// If no strategies are being passed, all strategies are eligible.
 		if ( null === $eligible_strategies ) {
 			$eligible_strategies = $this->delayed_strategies;
@@ -1053,9 +1058,9 @@ private function filter_eligible_strategies( $handle, $eligible_strategies = nul
 				return array();
 			}
 
-			$eligible_strategies = $this->filter_eligible_strategies( $dependent, $eligible_strategies, $checked );
+			$eligible_strategies = $this->filter_eligible_strategies( $dependent, $eligible_strategies, $checked, $stored_results );
 		}
-
+		$stored_results[ $handle ] = $eligible_strategies;
 		return $eligible_strategies;
 	}
 
@@ -1066,11 +1071,16 @@ private function filter_eligible_strategies( $handle, $eligible_strategies = nul
 	 * @see self::filter_eligible_strategies()
 	 * @see WP_Script_Modules::get_highest_fetchpriority_with_dependents()
 	 *
-	 * @param string              $handle  Script module ID.
-	 * @param array<string, true> $checked Optional. An array of already checked script handles, used to avoid recursive loops.
+	 * @param string                $handle         Script module ID.
+	 * @param array<string, true>   $checked        Optional. An array of already checked script handles, used to avoid recursive loops.
+	 * @param array<string, string> $stored_results Optional. An array of already computed max priority by handle, used to increase performance in large dependency lists.
 	 * @return string|null Highest fetch priority for the script and its dependents.
 	 */
-	private function get_highest_fetchpriority_with_dependents( string $handle, array $checked = array() ): ?string {
+	private function get_highest_fetchpriority_with_dependents( string $handle, array $checked = array(), array &$stored_results = array() ): ?string {
+		if ( isset( $stored_results[ $handle ] ) ) {
+			return $stored_results[ $handle ];
+		}
+
 		// If there is a recursive dependency, return early.
 		if ( isset( $checked[ $handle ] ) ) {
 			return null;
@@ -1099,7 +1109,7 @@ private function get_highest_fetchpriority_with_dependents( string $handle, arra
 		$highest_priority_index = (int) array_search( $fetchpriority, $priorities, true );
 		if ( $highest_priority_index !== $high_priority_index ) {
 			foreach ( $this->get_dependents( $handle ) as $dependent_handle ) {
-				$dependent_priority = $this->get_highest_fetchpriority_with_dependents( $dependent_handle, $checked );
+				$dependent_priority = $this->get_highest_fetchpriority_with_dependents( $dependent_handle, $checked, $stored_results );
 				if ( is_string( $dependent_priority ) ) {
 					$highest_priority_index = max(
 						$highest_priority_index,
@@ -1111,7 +1121,7 @@ private function get_highest_fetchpriority_with_dependents( string $handle, arra
 				}
 			}
 		}
-
+		$stored_results[ $handle ] = $priorities[ $highest_priority_index ]; // @phpstan-ignore parameterByRef.type (We know the index is valid and that this will be a string.)
 		return $priorities[ $highest_priority_index ];
 	}
 

tests/phpunit/tests/dependencies/scripts.php

@@ -1435,6 +1435,41 @@ public function test_fetchpriority_bumping_a_to_z() {
 		$this->assertEqualHTML( $expected, $actual, '<body>', "Snapshot:\n$actual" );
 	}
 
+	/**
+	 * Tests that `WP_Scripts::get_highest_fetchpriority_with_dependents()` correctly reuses cached results.
+	 *
+	 * @ticket 64194
+	 *
+	 * @covers WP_Scripts::get_highest_fetchpriority_with_dependents
+	 */
+	public function test_highest_fetchpriority_with_dependents_uses_cached_result() {
+		$wp_scripts = new WP_Scripts();
+		$wp_scripts->add( 'd', 'https://example.com/d.js' );
+		$wp_scripts->add_data( 'd', 'fetchpriority', 'low' );
+
+		/*
+		 * Simulate a pre-existing `$stored_results` cache entry for `d`.
+		 * If the caching logic works, the function should use this "high" value
+		 * instead of recalculating based on the actual (lower) value.
+		 */
+		$stored_results = array( 'd' => 'high' );
+
+		// Access the private method using reflection.
+		$method = new ReflectionMethod( WP_Scripts::class, 'get_highest_fetchpriority_with_dependents' );
+		if ( PHP_VERSION_ID < 80100 ) {
+			$method->setAccessible( true );
+		}
+
+		// Pass `$stored_results` BY REFERENCE.
+		$result = $method->invokeArgs( $wp_scripts, array( 'd', array(), &$stored_results ) );
+
+		$this->assertSame(
+			'high',
+			$result,
+			'Expected "high" indicates that the cached `$stored_results` entry for D was used instead of recalculating.'
+		);
+	}
+
 	/**
 	 * Tests that printing a script without enqueueing has the same output as when it is enqueued.
 	 *
@@ -1534,6 +1569,41 @@ public function test_loading_strategy_with_valid_blocking_registration() {
 		$this->assertSame( $expected, $output, 'Scripts registered with no strategy assigned, and who have no dependencies, should have no loading strategy attributes printed.' );
 	}
 
+	/**
+	 * Tests that `WP_Scripts::filter_eligible_strategies()` correctly reuses cached results.
+	 *
+	 * @ticket 64194
+	 *
+	 * @covers WP_Scripts::filter_eligible_strategies
+	 */
+	public function test_filter_eligible_strategies_uses_cached_result() {
+		$wp_scripts = new WP_Scripts();
+		$wp_scripts->add( 'd', 'https://example.com/d.js' );
+		$wp_scripts->add_data( 'd', 'strategy', 'defer' );
+
+		/*
+		 * Simulate a cached result in `$stored_results` for D.
+		 * If caching logic is functioning properly, this cached value
+		 * should be returned immediately without recomputing.
+		 */
+		$stored_results = array( 'd' => array( 'async' ) );
+
+		// Access the private method via reflection.
+		$method = new ReflectionMethod( WP_Scripts::class, 'filter_eligible_strategies' );
+		if ( PHP_VERSION_ID < 80100 ) {
+			$method->setAccessible( true );
+		}
+
+		// Invoke the method with `$stored_results` passed by reference.
+		$result = $method->invokeArgs( $wp_scripts, array( 'd', null, array(), &$stored_results ) );
+
+		$this->assertSame(
+			array( 'async' ),
+			$result,
+			'Expected cached `$stored_results` value for D to be reused instead of recomputed.'
+		);
+	}
+
 	/**
 	 * Tests that scripts registered for the head do indeed end up there.
 	 *