Speeding up Range.product_queryset()

[gwaidacher]

Speeding up Range.product_queryset()

see conversation in #4066 - created new PR with this issue.

[codecov]

Codecov Report

Merging #4100 (11a6f12) into master (eca7cee) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4100      +/-   ##
==========================================
+ Coverage   87.34%   87.41%   +0.07%     
==========================================
  Files         291      291              
  Lines       15825    15848      +23     
==========================================
+ Hits        13822    13854      +32     
+ Misses       2003     1994       -9     

Impacted Files	Coverage Δ
src/oscar/apps/offer/abstract_models.py	88.98% <100.00%> (+0.19%)	⬆️

... and 8 files with indirect coverage changes

[specialunderwear]

This is starting to look really good. I have on more suggestion, butt you need to verify if my remark makes sence, thanks a lot!

[gwaidacher]

It was not possible to save the queries for parent__categories and parent__excludes according to your suggestion, the changes failed in two different tests at self.assertTrue(self.range.contains_product(self.child1).

But with an additional check if Product.objects.filter(parent__excludes=self).exists(): it was possible to remove ~Q(parent__excludes=self) from the query, reducing query execution time from > 400 ms to < 4 ms:
We have more than 3,000.000 Products and a Range without classes and children; the additional check and the final query both had less than 2 ms execution time each.

[specialunderwear]

I didn't read the code correctly. Ignore this ...

bad reading of code

[specialunderwear]

Ok I'm going to test this on a site with lots of products and which uses child products.
There are a lot of queries added that do checks, so I'd like to know what the impact is on worst case performance before merging this.

I also have 1 more idea, maybe it is possible to do all the checks in a single query, using aggregations or something. Maybe you can look into that, in the meanwhile next week I'll try to find out how this works on a site with lot's of child products. Thanks for your effort!

[gwaidacher]

Ok I'm going to test this on a site with lots of products and which uses child products. There are a lot of queries added that do checks, so I'd like to know what the impact is on worst case performance before merging this.

I also have 1 more idea, maybe it is possible to do all the checks in a single query, using aggregations or something. Maybe you can look into that, in the meanwhile next week I'll try to find out how this works on a site with lot's of child products. Thanks for your effort!

I executed the checks in shell, they take no time (< 2ms) each. I tested it with a Range having included_products and a Range having included_categories.

I could not find any useful information on howto perform multiple checks for existence on different fields using aggregate functions.

Joining the checks on different fields again would create something like the original query over categories, parents and classes I want to avoid with this PR. Remember, we had a response time over 10 seconds, having a simple range without classes and included_categories, because the Products table was joined multiple times (... from catalogue_product T1 join catalogue_product T2 on ... ) although these joins were not necessary.

Conclusion: In my opinion, even if it would be possible to do all the checks in a single query (e.g. with a raw query) this would not be faster than the single checks with a huge product database.

I expect you will get some positive results concerning performance with your tests, the query having child products should also be faster if there are no classes or no included_categories in the range.

[gwaidacher]

Ok I'm going to test this on a site with lots of products and which uses child products. There are a lot of queries added that do checks, so I'd like to know what the impact is on worst case performance before merging this.

The two checks if the range has classes or included_categories can be ignored, they check if range_id exists on indexed field range_id on rather small tables offer_range_classes and offer_range_included_categories, so these checks will always be very fast.

The other checks join the product_ids from the range's included or excluded products, it's classes or it's included_categories on catalogue_product.parent_id which is also an indexed field - in each single case it takes only a few milliseconds in my database.

... FROM "catalogue_product" INNER JOIN "catalogue_product" T2 ON ("catalogue_product"."parent_id" = T2."id") INNER JOIN "offer_rangeproduct" ON (T2."id" = "offer_rangeproduct"."product_id") ...

The worst case, having a range with child products, classes and categories where all checks are performed might consume an average additional time between 10 and 20 ms for all checks, which can be ignored because the worst case overall query will take multiple seconds to be executed if there are millions of products in the database.

[specialunderwear]

@gwaidacher I just tested this in a site that is very heavy on child products and I got very impressive results, I'm going to try to do all the checks in one query. But this is very good allready!

[specialunderwear]

I decided the added complexity of doing conditional queries is not advantageous and to leave you PR as is, with minor readability changes.