2.4 performance

1M is pretty big :-)

I know about LINQ generating SQL and you're right - it's slow. That is why NopCommerce went back to stored procedures in the first place (good move!). Anyway one would suspect that hand crafted SQL has the potential to be be faster :-)

The question remains wheather we want to ditch a bunch of parameters on a universal stored procedure or do we want to use additional super optimised stored procedures:
ProductLoadAllPaged_CatalogNoParams - optimised for plain category browsing,  
ProductLoadAllPaged_Tag - optimised for showing tags,
ProductLoadAllPaged - the current universal slow one with tags taken out since they're already taken care of ...

Of course a mixed approach is possible where SQL side dynamic SQL is used in ProductLoadAllPaged to speed things up..

Filip

Also few more thoughts ..

Maybe NopCommerce should do a bit of denormalising ..

There are SpecialPriceStartDateTimeUtc and AvailableStartDateTimeUtc columns in the database on the ProductVariant table that are used in filtering .. maybe we could make a PublishedNow and PriceNow column and populate it periodically (task) from the special price and available start time columns .. This way we could ditch a lot of complex looking code in favor or just  filtering on the PublishedNow column ..  

Note: NopCommerce is already simplifying things a bit .. when sorting by price it doesn't take into account special prices .. the results can be wrong but still it is the right way forward IMHO ..

Also I wonder about Multiple manufacturers per product feature .. it causes us to have a whole new many-to-many table to join which I suspect slows things down further .. I can't really think about many real life products that have more than one manufacturer .. maybe it would be running a poll or finding out how many people are actually using it .. also .. since we already have tags which are already many-to-many we could use this for manufacturers too .. for example with simple modification to tag table (addition of a TagType) I was able to implement fairly easily linking of products by publisher (aka manufacturer), author, publishing series etc. (see http://ksiegarniainternetowa.de/p/100685/jak-w-niebie for example)

Filip

fkierzek wrote:
It's too complex. It's more like a performance hack for me

fkierzek wrote:
nopCommerce official site uses it here(versions are manufacturers) =)))

fkierzek wrote:
I also thought about it. Actually we could have only one table [Catalog] instead of three [Category], [Manufacturer] and [ProductTag]. Just add a new enum property [CatalogType] which will indicate a catalog type (category, manufacturer, or product tag). In it would be quite easy to add any new type such such department, etc.

[quote]
Actually we could have only one table [Catalog] instead of three [Category], [Manufacturer] and [ProductTag]. Just add a new enum property [CatalogType] which will indicate a catalog type (category, manufacturer, or product tag). In it would be quite easy to add any new type such such department, etc.
[/quote]

Just one table to join instead of 3. Nee .. that sounds like it would work too fast ;-)

OK - back to serious. I see a few advantages to this approach:
-> performance (less joins to do)
-> less code (same code to do handle edit/display of Categories Manufacturers and Tags)
-> more flexibility e.g. currently it's not possible to filter on tag and price or other attribute simultaneously
-> tag pictues and descriptions just like for manufactuers (good for SEO)
-> if we made URL that would cause only one of category types (eg. books / DVDs / Music CD) to show on home page we would effectively implement multiple category trees (i've seen requests for this few times)

I suspect that this is just the tip of an iceberg ..

Things to think about:
-> sub-manufacturers and sub-tags? would we prohibit these or allow them?
-> category/tag/manufactuer URLs .. currently these overlaping ID numbers .. these would need to be renumbered to be unique if thrown into one bag .. or would they? we could use a different routes to get to different CatalogTypes ..

Filip

Agree, there are some advantages and disadvantages. Of course sub-manufacturers and sub-tags won't be allowed. And overlapping IDs could be the issue for existing store owners. Not sure that it'll be implemented in the near time (that was just a thought). Although I created a proposed work item here

Have you guys tried refactoring the OUTER JOINS into EXISTS?   The execution plan is radically different.
My database isn't really big enough to see whether this really improves performance, and I don't have the time at the moment to fabricate a bigger one.  With my 5000 product DB, I see about a 3x improvement for this subset of the SQL, though going from .3 to .89 seconds may not get comparable saving in the real world.  Note also, that I changed the OUTER JOIN on the ProductVariant to just a regular JOIN.  


    DECLARE
      @CategoryId      int = 0,
      @ManufacturerId    int = 0,
      @ProductTagId    int = 0,
      @FeaturedProducts  bit = null,  --0 featured only , 1 not featured only, null - load all products
      @Keywords      nvarchar(MAX) = 'TEST', --null,
            @SearchDescriptions bit = 0
        ;
  
  
    
  DECLARE @SearchKeywords bit
  SET @SearchKeywords = 1
  IF (@Keywords IS NULL OR @Keywords = N'')
    SET @SearchKeywords = 0  

  SELECT p.Id
  FROM Product p with (NOLOCK)
  JOIN ProductVariant pv with (NOLOCK) ON p.Id = pv.ProductId

  WHERE     
       (
        @CategoryId IS NULL OR @CategoryId=0
        OR EXISTS (SELECT 1 FROM Product_Category_Mapping pcm with (NOLOCK)
                   WHERE p.Id=pcm.ProductId
                     AND pcm.CategoryId=@CategoryId
                     AND (@FeaturedProducts IS NULL OR pcm.IsFeaturedProduct=@FeaturedProducts))
      )
    AND (
        @ManufacturerId IS NULL OR @ManufacturerId=0
        OR EXISTS (SELECT 1 FROM Product_Manufacturer_Mapping pmm with (NOLOCK)
                   WHERE p.Id=pmm.ProductId
                     AND pmm.ManufacturerId=@ManufacturerId AND (@FeaturedProducts IS NULL OR pmm.IsFeaturedProduct=@FeaturedProducts))
      )
    AND (
        @ProductTagId IS NULL OR @ProductTagId=0
        OR EXISTS (SELECT 1 FROM Product_ProductTag_Mapping pptm with (NOLOCK)
                   WHERE p.Id=pptm.Product_Id
                     AND pptm.ProductTag_Id=@ProductTagId)
      )
      
    AND  (
        @SearchKeywords = 0 or
        (
          -- search standard content
          patindex(@Keywords, p.name) > 0
          or patindex(@Keywords, pv.name) > 0
          or patindex(@Keywords, pv.sku) > 0
          or (@SearchDescriptions = 1 and patindex(@Keywords, p.ShortDescription) > 0)
          or (@SearchDescriptions = 1 and patindex(@Keywords, p.FullDescription) > 0)
          or (@SearchDescriptions = 1 and patindex(@Keywords, pv.Description) > 0)          
        )
      )

New York wrote:

NY - I think you've hit the nail on this one. We need larger set of data available to make us realize problems with the current design. If a 20 second page load times don't hit you in the face like an 18 wheeler you won't realize there is a problem. Additionally we need some attributes in that dataset. Currently the only category that has attributes we can filter on is the notebooks category (http://demo.nopcommerce.com/c/4/notebooks) on the demo data set. The category has only 8 products .. which isn't realistic for testing of filtering.

How about a PL/SQL script to generate a random such of data? .. I am thinking about a script that would get Andrey 10.000 new cell phones to sell ;-). I am thinking that each cell phone would need an operating system (iOS, Bada, Andorid) , camera with different megapixels, wifi and mp3 player (or not) and would have a price. The phones wouldn't get installed by default  - you would have to execute a stored procedure to create them.

Than we could talk about whether EXISTS vs OUTER (or not) JOIN makes things go faster or not.

Filip

PS. NY - I am not able to test your suggestion on a larger dataset as I had to take out  most of the joins out completely just to get Nop 2.4 to work in commercially acceptable fashion.

When looking at the c# code another thing springs to mind. The stored precedure returns just the product columns discarting everything else. Yet the very next thing the apps needs to do is to prepare the ProductOverviewModel which includes the PictureModel and in many (most?) cases the PriceModel. This causes more database accesses to happen the very next moment to pull the corresponding product variants and product pictures..

What would happen if we returned a product + variant (minimum priced one if more than 1 variant exists) + primary picture row directly from the stored procedure? LINQ to SQL handles the custom types easily so this would go a long way to reducing the amount of further SQL calls that need to be made..

Granted some of these calls are cached etc. etc. but why cache the additional call when you can not make it instead?

Filip

fkierzek wrote:
I used the following script (for version 2.40). It's not very elegant (just for internal usage) and it doesn't add any attribute mappings. 1 product, 1 variant, 1 category and 1 manufacturer (the most common scenario). Although the number of product tags could be increased to 3-4.

DECLARE @Flag INT
  SET @Flag = 1
WHILE (@Flag < 1)
  BEGIN
  declare  @productid int
  insert [dbo].[Product]([Name], [ShortDescription], [FullDescription], [AdminComment], [ProductTemplateId], [ShowOnHomePage], [MetaKeywords], [MetaDescription], [MetaTitle], [SeName], [AllowCustomerReviews], [ApprovedRatingSum], [NotApprovedRatingSum], [ApprovedTotalReviews], [NotApprovedTotalReviews], [Published], [Deleted], [CreatedOnUtc], [UpdatedOnUtc])
  values ('test' + CONVERT(NVARCHAR(20), @Flag), 'short test description' + CONVERT(NVARCHAR(20), @Flag), 'full test description' + CONVERT(NVARCHAR(20), @Flag), null, 1, 0, null, null, null, null, 1, 0, 0, 0, 0, 1, 0, getutcdate(), getutcdate())
  select @productid=scope_identity()
  --category
  DECLARE @RandomCategoryId int
  SELECT @RandomCategoryId = 10 * RAND() + 1
  insert [dbo].[Product_Category_Mapping]([ProductId], [CategoryId], [IsFeaturedProduct], [DisplayOrder])
  values (@productid, @RandomCategoryId, 0, @RandomCategoryId)
  --manufacturer
  DECLARE @RandomManufacturerId int
  SELECT @RandomManufacturerId = 2 * RAND() + 1
  insert [dbo].[Product_Manufacturer_Mapping]([ProductId], [ManufacturerId], [IsFeaturedProduct], [DisplayOrder])
  values (@productid, @RandomManufacturerId, 0, @RandomManufacturerId)
  --tag
  DECLARE @RandomProductTagId int
  SELECT @RandomProductTagId = 16 * RAND() + 1
  insert [dbo].[Product_ProductTag_Mapping]([ProductTag_Id], [Product_Id])
  values (@RandomProductTagId, @productid)
  --variant
  insert [dbo].[ProductVariant]([ProductId], [Name], [Sku], [Description], [AdminComment], [ManufacturerPartNumber], [Gtin], [IsGiftCard], [GiftCardTypeId], [RequireOtherProducts], [RequiredProductVariantIds], [AutomaticallyAddRequiredProductVariants], [IsDownload], [DownloadId], [UnlimitedDownloads], [MaxNumberOfDownloads], [DownloadExpirationDays], [DownloadActivationTypeId], [HasSampleDownload], [SampleDownloadId], [HasUserAgreement], [UserAgreementText], [IsRecurring], [RecurringCycleLength], [RecurringCyclePeriodId], [RecurringTotalCycles], [IsShipEnabled], [IsFreeShipping], [AdditionalShippingCharge], [IsTaxExempt], [TaxCategoryId], [ManageInventoryMethodId], [StockQuantity], [DisplayStockAvailability], [DisplayStockQuantity], [MinStockQuantity], [LowStockActivityId], [NotifyAdminForQuantityBelow], [BackorderModeId], [AllowBackInStockSubscriptions], [OrderMinimumQuantity], [OrderMaximumQuantity], [DisableBuyButton], [DisableWishlistButton], [CallForPrice], [Price], [OldPrice], [ProductCost], [SpecialPrice], [SpecialPriceStartDateTimeUtc], [SpecialPriceEndDateTimeUtc], [CustomerEntersPrice], [MinimumCustomerEnteredPrice], [MaximumCustomerEnteredPrice], [Weight], [Length], [Width], [Height], [PictureId], [AvailableStartDateTimeUtc], [AvailableEndDateTimeUtc], [Published], [Deleted], [DisplayOrder], [CreatedOnUtc], [UpdatedOnUtc])
  values (@productid, null, null, null, null, null, null, 0, 0, 0, null, 0, 0, 0, 0, 0, null, 0, 0, 0, 0, 
  null, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10000, 0, 0, 0, 0, 1, 0, 0, 
  1, 10000, 0, 0, 0, @productid, 0, 0, null, null, null, 0, 0, 0, 0, 0, 0, 0, 0, null, null, 1, 0, @productid, getutcdate(), getutcdate())

  SET @Flag = @Flag + 1
END
GO

P.S. 10, 2, 16 used in @RandomCategoryId, @RandomManufacturerId, @RandomProductTagId initialization are the maximum available identifiers on my sample database.

Excellent!

Thanks for the script Andrei - I'll see if I can make a phone factory out of it :-)

Filip