Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issues compared to original implementation #131

Open
leewillis77 opened this issue Oct 25, 2018 · 10 comments
Open

Performance issues compared to original implementation #131

leewillis77 opened this issue Oct 25, 2018 · 10 comments

Comments

@leewillis77
Copy link

I've been doing some testing with the Google Product Feed extension (https://woocommerce.com/products/google-product-feed). Since this has to iterate through all store products to format and output them it's a reasonable stress-test for things like this.

The implementation takes care to query for things using appropriate functions to maximize the use of WordPress' object cache and minimize the number of database queries that have to be run.

After my first round of compatibility work (making sure that the output is consistent between the standard / custom tables implementations) I've started to look at performance.

Here's my initial findings (with a product set of 4 products / 131 variations in total):

Standard implementation:
~670ms - 594 DB queries

Custom tables implementation:
~1150ms - 1510 DB queries

As you can see, moving to the custom table implementation results in 2.5x the number of DB queries, and a 1.8x slowdown.

I've attached a list of queries generated when accessing the feed with the standard implementation and by the custom tables implementation for reference. Could someone familiar with the custom tables internals take a look and suggest if that's what you'd expect?

current-queries.txt
custom-tables-queries.txt

@leewillis77
Copy link
Author

Just following up on this, I'm seeing query-explosions with the WooCommerce MSRP plugin as well which is a lot simpler. Just viewing a product page has a huge performance slowdown:

Standard implementation:
184ms, 146 queries

Custom tables implementation:
1017ms, 1537 queries

I'm going to do some digging in my code to see if I can work out which code / call is triggering the behaviour.

@leewillis77
Copy link
Author

The issues I'm experiencing are on variable products (specifically I have a product with 131 variations on it).

For the MSRP plugin, the current stable release has the following code [simplified for clarity] to get the lowest / highest MSRPs for a product:

$children = $current_product->get_children();
foreach ( $children as $child_id ) {
    $child_msrp = get_post_meta( $child_id, '_msrp', true );
}

The product page on a standard install with that code:

  • 237ms
  • 185 DB queries
  • 16MB of memory usage.

In order to stop using post meta functions I've updated this on my dev build to:

$children = $current_product->get_children();
foreach ( $children as $child_id ) {
    $child_product = wc_get_product( $child_id );
    $child_msrp = $child_product->get_meta( '_msrp' );
}

The product page on a standard install with that code:

  • 500ms
  • 569 DB queries
  • 22MB of memory.

Activating the custom tables plugin means that that code:

  • 853ms
  • 1447 DB queries
  • 61MB of memory.

A lot of the impact comes from the fact that I'm now loading the whole product versus just reading a meta value - so if there's a supported way of doing that in the new world then that will probably help my use cases here.

Performance figures from Time Stack plugin on WooCommerce 3.5 with the latest build of this plugin.

@pmgarman
Copy link

pmgarman commented Mar 8, 2019

@leewillis77 this has been around for a while, any word on your side if there is any improvement or feedback from the dev team?

@leewillis77
Copy link
Author

Hi Patrick;

There's no change (yet) as far as I know.

@claudiosanches
Copy link
Contributor

@leewillis77 have you tested with the compatibility layer deactivated?

You can disable by including it on your wp-config.php:

define( 'WC_PRODUCT_TABLES_DISABLE_BW_COMPAT', true ); // Need to remove in order to enable again.

I'm seen some performance issues due to the compatibility layer trying to update products on the fly.

Thanks for your feedback.

@wss-chadical
Copy link

@claudiosanches Interesting switch there. Would it make sense to do all our testing with this off?

@claudiosanches
Copy link
Contributor

@wss-chadical I'm going to remove the backwards compatibility layer soon, since the performance cost is high. If you like to test, probably better to disable it anyway.

@leewillis77
Copy link
Author

Thanks for the feedback @claudiosanches

I wasn't aware of that flag, but can definitely re-test with that disabled. Unfortunately I've cleaned everything down since opening this so will need to set up a suitable test environment again. I'll update here next week when I've had chance to test.

@leewillis77
Copy link
Author

Hi @claudiosanches

Disabling the back-compat layer doesn't make any difference to the queries run.

I've run tests on a single product page view of a variable product with 120 variations & MSRP plugin active with each of the following scenarious:

  1. Standard WooCommerce + Current MSRP plugin release
  2. Standard WooCommerce + Modified MSRP plugin (rewrite get_post_meta() calls to $product->get_meta())
  3. WooCommerce + Product Tables Feature Plugin + Modified MSRP plugin
  4. WooCommerce + Product Tables Feature Plugin + Modified MSRP plugin (Back-compat disabled)

The results are:

  1. 201 queries
  2. 576 queries
  3. 1530 queries
  4. 1530 queries

Even the jump from [1] to [2] is worrying and stems from the fact that there's no longer a way to retrieve a meta value for a product without instantiating the whole product object which is a design issue I think?

The jump between [2] and [3] seems like it's maybe just an implementation issue that could perhaps be solved with object caching or similar?

@claudiosanches
Copy link
Contributor

@leewillis77 thanks you very much for test again, this will help me a lot fixing those issues.

Even the jump from [1] to [2] is worrying and stems from the fact that there's no longer a way to retrieve a meta value for a product without instantiating the whole product object which is a design issue I think?

Yes, because the new implementation, and we should improve it with custom tables for sure.

The jump between [2] and [3] seems like it's maybe just an implementation issue that could perhaps be solved with object caching or similar?

There's already some caching for objects, but for sure we are failing to proper handle it, I'll work on it. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants