soflyy / breakdance-bugs

Bug reports from Breakdance users.
38 stars 5 forks source link

Poor Performance with Woo "wc_get_template_part" #1065

Open hagedigital opened 1 month ago

hagedigital commented 1 month ago

Originally reported via breakdance.com support Thu, 20 Mar 2024. Bugged responded to by Elijah @ Breakdance 24 March 2024 with no estimated time for resolution "I'm sorry, but we don't provide estimates or timelines for fixes."

The wc_get_template_part function in WooCommerce does handle some caching, but the code you've used to override the template replaces the default behavior with a custom function that doesn't implement caching. Your custom filter is modifying the template path based on the $slug and $name parameters, and then using file_exists to check for the file's existence in the specified directory.

Snippet found in breakdance/subplugins/breakdance-woocommerce/util/templates.php

/*
 * wc_get_template_part handles caching
 * i don't think this code does though. it looks like it just calls load_template on the returned path
 * does that cache? i don't think so.
 * so we are calling load template for the same file 27 times in a lot of cases
 (shop loop, for example)
-*/

add_filter(
    "wc_get_template_part",
    /**
     * @param string $template
     * @param string $slug
     * @param string|null $name
     * @return string
     */
    function($template, $slug, $name) {
        if (defined('DEBUG_WOO_TEMPLATES') && DEBUG_WOO_TEMPLATES) {
            debugTemplate($template, $slug, $name);
        }

        $file = $name ? "{$slug}-{$name}.php" : "{$slug}.php";
        $path = BREAKDANCE_WOO_TEMPLATES_DIR . $file;
        return file_exists($path) ? $path : $template;
    },
    10, 3
);

Your original code has a few potential drawbacks:

Lack of Caching:

Each time the filter is applied, file_exists is called to check if the template file exists in the specified directory. This check can be relatively expensive, especially if it's done repeatedly in a loop (like in the shop loop). Repeated File System Access:

Since there is no caching mechanism, the file system is accessed every time the filter runs, which can significantly impact performance, particularly on busy sites with many products. Potential Debug Overhead:

If DEBUG_WOO_TEMPLATES is enabled, the debugTemplate function is called every time the filter runs. This can add extra overhead if it's logging or processing a lot of information. Inconsistent Performance:

In cases where the template file does not exist in the specified directory, the code falls back to the default template, but this check happens every time the filter is applied, leading to consistent but unnecessary performance overhead. Scalability Issues:

On a site with many products or complex templates, the repeated execution of file_exists and the lack of caching can scale poorly, resulting in slower page load times. Here’s a more detailed look at each issue:

Lack of Caching Without caching, the code calls file_exists every time wc_get_template_part is invoked, even if the same template file is needed multiple times in a single page load. This can be particularly problematic in loops, where the same template might be required for each iteration.

Repeated File System Access The file_exists function involves accessing the file system, which is generally slower than accessing data from memory. Repeated calls to file_exists can slow down the overall performance of the site.

Potential Debug Overhead When debugging is enabled, the debugTemplate function is called. Depending on what this function does, it could add significant overhead, especially if it performs logging or other resource-intensive operations.

Inconsistent Performance Each time the template is needed, the code performs the same checks and operations. In the worst case, if the custom template file does not exist, the file_exists check is unnecessarily repeated, wasting resources.

Scalability Issues On a high-traffic site, the lack of caching can lead to performance bottlenecks. As the number of products or complexity of the templates increases, the negative impact on performance can become more pronounced.

Summary To improve the performance and address these issues, a simple caching mechanism can be introduced, as shown in the revised code provided earlier. This caching mechanism minimizes file system access, improves performance, and scales better with increasing site complexity.

If you have any specific performance metrics or scenarios you are concerned about, we can further fine-tune the solution to better meet those needs.

If you want to implement caching to avoid calling load_template multiple times for the same file, you can introduce a simple caching mechanism within your custom filter. Here's an example of how you could do that:

add_filter(
    "wc_get_template_part",
    /**
     * @param string $template
     * @param string $slug
     * @param string|null $name
     * @return string
     */
    function($template, $slug, $name) {
        static $template_cache = [];

        $cache_key = $slug . ($name ? "-$name" : '');

        if (isset($template_cache[$cache_key])) {
            return $template_cache[$cache_key];
        }

        if (defined('DEBUG_WOO_TEMPLATES') && DEBUG_WOO_TEMPLATES) {
            debugTemplate($template, $slug, $name);
        }

        $file = $name ? "{$slug}-{$name}.php" : "{$slug}.php";
        $path = BREAKDANCE_WOO_TEMPLATES_DIR . $file;

        if (file_exists($path)) {
            $template_cache[$cache_key] = $path;
        } else {
            $template_cache[$cache_key] = $template;
        }

        return $template_cache[$cache_key];
    },
    10, 3
);

Explanation: Static Variable for Caching:

static $template_cache = []; is a static variable that persists across function calls, used here to store the template paths. Cache Key:

$cache_key is constructed from the $slug and $name parameters to uniquely identify each template file. Check Cache:

if (isset($template_cache[$cache_key])) checks if the template path is already cached. If so, it returns the cached path. File Check and Caching:

file_exists($path) checks if the file exists in the custom directory. The result is stored in the cache ($template_cache[$cache_key]), whether the custom file exists or not. Return Cached Path:

Finally, the cached path is returned, ensuring that load_template is not called multiple times for the same file. This approach ensures that the load_template function is not called multiple times for the same template, thereby improving performance by utilizing a simple caching mechanism.

Breakdance 1.7.2 & Breakdance 2.0.0 beta Chrome/Safari/Arc. Mac/Windows