2.4 performance

Hi,

NopCommerce made leaps in terms of performance with 2.4 release. I've noticed this in terms of amount of coding I have to spend making sure the NopCommerce is usable performance wise in a big store (70.000+ products) and with 2.4 I've noticed it wasn't very complicated to get it work much more nicely.

No point however of me doing it again with nop 2.5 when it can be done on the open source code for everyone's benefit. So I am writing this to say what I've done / what I think can be done so that it can be discussed and than a fork created to put these changes into main branch. So here it goes.

===

NopCommerce moved some of the product selection logic to ProductLoadAllPaged() stored procedure when used on Sql Server database. Great move! However the procedure is a all-in-one swiss army knife that doesn't really do specific things optimally. For example:

1. It checks all the attribute filtering logic even though there aren't any - and it has been turned off via an option in the panel. This is nicely IF-ed out in LINQ to SQL but not in stored procedure. Sql profiler says 20% of time is spent on that in my store that doesn't have any attributes.

2. Showing products for a tag. I think this needs to be a separate procedure. We don't need manufacturers, attribute filtering or text searching here or even categories. At least I haven't found a place where I could filter via tag and something else..

3. Showing products in category. Again basic category browsing is pretty simple - join product to category via product-to-category table and order thaem and  that's it. So a basic "ProductLoadFromCategoryWithNoFiltering' procedure would be much faster in most of the cases where manufacturers, price ranges and keyword searches are not needed. This would need to be IF-ed in the c# code to call the right one depending if there are params or not.

4. I wander about using stored procedures only for front end and not for back end. In front end for example we don't show products that are not published or deleted whilst in back end we need to. So if we left back end using the LINQ to SQL approach and used the stored procedures only in front end we could cut all the   >> IF @ShowHidden = true << fat out of them ..

===

The list of bestsellers on home page was (before I kicked it out) doubling home page load time. This should of course OutputCache-ed but even if it can be slow the first time. Suggested approach: add a column sales rank column (indexed of course) which would order the products without joining all the other order related tables in the universe. This could be updated periodically via Task. Of course once we have this it can be shown in the front end on product page as an additional bonus to the customer.

Alternatively this column could record number of times given product made it into an order.



===

Additionally a Bunch of indexes is needed. There are some foreign key columns that beg to be indexed (eg. productid in productvariant table) and some frequently used columns like Product.deleted and product.published. There is also a lot of indexes missing on the tables that have to do with product attributes but I haven't worked these out yet since I am not using that part of nop commerce at the moment.

Some of the indexes I've added to make things work a bit faster:

CREATE NONCLUSTERED INDEX [ITIDEA_Product_Deleted_and_Published] ON [dbo].[Product]
(
  [Published] ASC,
  [Deleted] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
GO

CREATE NONCLUSTERED INDEX [ITIDEA_Product_Published] ON [dbo].[Product]
(
  [Published] ASC
)
INCLUDE ( [Deleted]) WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
GO

CREATE NONCLUSTERED INDEX [ITIDEA_Product_ShowOnHomepage] ON [dbo].[Product]
(
  [ShowOnHomePage] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
GO

CREATE NONCLUSTERED INDEX [ITIDEA_ProductVariant_DisplayOrder] ON [dbo].[ProductVariant]
(
  [DisplayOrder] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
GO

CREATE NONCLUSTERED INDEX [IX_ProductVariant_ProductId] ON [dbo].[ProductVariant]
(
  [ProductId] ASC
)

CREATE CLUSTERED INDEX [ITIDEA_PCM_Product_and_Category] ON [dbo].[Product_Category_Mapping]
(
  [CategoryId] ASC,
  [ProductId] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
GO

All right - so here it is. Fellow NopCommerce-ers - I am awaiting your (hopefully constructive) criticism and opinions :-)

Filip

Nice suggestions Filip. However there is already some performance optimization under way for v2.5 here:

http://nopcommerce.codeplex.com/workitem/10283

So I took the freedom link it to your post.

Doesn't look like there is much activity on the workitem, other than the comment by AM that it slows things down.

Also I am a fan of not doing unnecessary joins/where's rather than doing them in a more optimal way.

Filip

Filip,

Thanks a lot. Although I agree that the stored procedure does too much work, but it's quite easy to maintain such "universal" procedure. Maybe, using dynamic SQL (based on conditions) would be better. It'll also to have only one stored procedure and simplify the code (see here). I'll also create work items for the other things you suggested.

fkierzek wrote:
BTW,
1. ITIDEA_ProductVariant_DisplayOrder index already exists
2. IX_ProductVariant_ProductId index name is already reserved (I renamed this one)
See changeset 6ce9356e1ef7

a.m. wrote:

I have mixed feeling about Dynamic Sql. First of all I think it's more straightforward to program in C# than in stored precedures so we could pretty much just do the same dynamic SQL in C# instead. Also maintenance wise I think it is much bigger problem than static SQL due to complexity. It reminds me of the of times I wrote specialized string functions to manipulate sql statments and the fun of finding out that the application broke cause there is a missing comma somewhere in the sql code my methods generated..

I've read the above article including the post by NewYork, and he actually seems to agree:
NewYork wrote:

There is definitely more to carry when you have a box of specialized tools rather than single swiss army knife. However a standalone pair of scissors is a better pair or scissors than the one found in a swiss army knife. I didn't want to create a specialized stored procedures for tags because I like to change things that work but because it just took to long to show results.

I can help out with the code but I rather agree on general direction first.

Filip

PS. BTW Andrei - is my Google Analytics contribution any good?
http://nopcommerce.codeplex.com/SourceControl/network/Forks/fkierzek/GoogleAnalyticsEcommerceTracking/contribution/1828

fkierzek wrote:
Sure, any help is always welcome. But I still haven't decided what approach is better (still thinking)

fkierzek wrote:
I saw the pull request and haven't had time to review it yet. This work item is planned for version 2.50. I'll review it in the near time.

All right - let me know once you've thought about it.

While talking about stored procedures and maintenance .. this looks like a bug:

  ORDER BY
    CASE WHEN @OrderBy = 0 AND @CategoryId IS NOT NULL AND @CategoryId > 0
    THEN pcm.DisplayOrder END ASC,
    CASE WHEN @OrderBy = 0 AND @ManufacturerId IS NOT NULL AND @ManufacturerId > 0
    THEN pmm.DisplayOrder END ASC,
    CASE WHEN @OrderBy = 0 THEN p.[Name] END ASC,
    CASE WHEN @OrderBy = 5

shouldn't

CASE WHEN @OrderBy = 0 THEN p.[Name] END ASC,

be

CASE WHEN @OrderBy = 0 THEN pv.[DisplayOrder] END ASC,

I would think after all that our intention was to sort by 'position' and not alphabetlically..

    /// <summary>
    /// Represents the product sorting
    /// </summary>
    public enum ProductSortingEnum
    {
        /// <summary>
        /// Position (display order)
        /// </summary>
        Position = 0,

Filip

No, it's OK. Position sorting is used only when you're on category or manufacturer details page (previous two CASE rows - "CASE WHEN @OrderBy = 0 AND @CategoryId IS NOT NULL AND @CategoryId > 0"). Otherwise, we sort it by name.

fkierzek wrote:
If stored procedures are not supported, then required SQL code is generated by LINQ using C# (just set 'commonsettings.usestoredproceduresifsupported' setting to 'false'), but the SQL command generated by Entity Framework is not so elegant and fast =(((

BTW, I have tested the suggested solution (dynamic SQL) again with almost 1M products and it works 2-3 times faster than the original one. Although it's still slow when you don't have so many products.