Make WordPress Core

Opened 19 months ago

#56909 new defect (bug)

pre_recurse_dirsize filter cannot be used to fill up dirsize_cache and thus breaks performance

Reported by: janthiel's profile janthiel Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.6
Component: Networks and Sites Keywords: 2nd-opinion dev-feedback needs-patch
Focuses: multisite, performance Cc:

Description

In 5.6.0 a new filter was introduced to the dirsize calculation pre_recurse_dirsize. After that filter was introduced the dirsize cache was modified to store each folders size separately for a massive performance increase ( part of https://core.trac.wordpress.org/ticket/19879 ).

https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/functions.php#L8287

This second change lead to a state where the pre_recurse_dirsize filter is kind of useless. One cannot access or modify the dirsize cache within the filter as the $dirsize_cache variable is passed by reference to the recursive calls of recurse_dirsize().

https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/functions.php#L8300

Thus using the pre_recurse_dirsize filter renders it impossible to use the new, much more efficient dirsize cache based on single folders. I can only fill up the total for the top level folder.

If pre_recurse_dirsize is used the code would skip these recursive calls to recurse_dirsize(). And thus the reference passing of the $dirsize_cache and filling it with the subfolder sizes.

One would consider that the filter code could set the $dirsize_cache or the transient value on its own. This doesn't work as well, as the original code works on an in memory version of the $dirsize_cache and will overwrite any changes done within the filter at the end of its code.

https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/functions.php#L8323-L8328

This state leads to the bad situation that using the pre_recurse_dirsize filter will always lead to worse performance. Although the idea behind introducing it was to open up for performance improvements.

I am currently unsure how to fix this in a smart way and am open for any thoughts and suggestions.

(Maybe bad) Ideas I had:

  • Pass the $dirsize_cache by the reference to the filter (technically impossible as far as I know)
  • Add another filter to disable the dirsize cache saving in recurse_dirsize to handle everything on our own (would allow full backward compat)
  • Move the pre_recurse_dirsize to another position (don't really know where...)
  • Make recurse_dirsize a pluggable function to replace it completely

Thanks a lot!

Change History (0)

Note: See TracTickets for help on using tickets.