Jared,
That's great input regarding the tuning of the existing stored procedure and is definitely a quick fix.
However, in the next release of nopCommerce I would like to see us move away from stored procedures completely (unless absolutely necessary).
I have already discussed how we can flex IQueryable for a better paging solution (see my post here - http://blogs.planetcloud.co.uk/mygreatdiscovery/post/Efficient-GridView-paging-with-IQueryable.aspx).
By taking advantage of deferred query execution and some refactoring we can certainly improve performance.
The first thing I would recommend is we get rid of this:
public static List<Product> GetAllProducts(int categoryId,
int manufacturerId, int productTagId, bool? featuredProducts,
decimal? priceMin, decimal? priceMax,
int relatedToProductId, string keywords, bool searchDescriptions, int pageSize,
int pageIndex, List<int> filteredSpecs, int languageId,
ProductSortingEnum orderBy, out int totalRecords) { .... yuck .... }
The problem with making a method like this so generic is that you just end up with many overloads. It also makes caching a nightmare (ref:Deval) - try coming up with a cache key for this one. As you can see currently, we are not even caching these result.
Instead we should refactor into more specific methods that can return a paged list (see the link above for a pagedlist implementation).
For example:
public static PagedList<Product> GetProductsInCategory<TKey>(int categoryId,
decimal? priceMin, decimal? priceMax, int pageSize,
int pageIndex, Expression<Func<Product, TKey>> orderBy) {
var db = ObjectContextHelper.CurrentObjectContext;
var query = from p in db.Products
from pc in p.NpProductCategories
where pc.CategoryId == categoryId
select p;
if (priceMin.HasValue && priceMax.HasValue)
{
query = from p in query
from pv in p.NpProductVariants
where pv.Price <= priceMax.Value &&
pv.Price >= priceMin.Value
select p;
}
return new PagedList<Product>(query.OrderBy(orderBy), pageIndex, pageSize);
}
Usage:
var productCollection = ProductManager.GetProductsInCategory<string>(category.CategoryId,
minPriceConverted, maxPriceConverted, pageSize, this.CurrentPageIndex, x => x.Name);
Okay so we can make this method better, but you get the idea.
Next, recommendation, get a copy of
EF Prof so you can see exactly what SQL is generated. When it comes to ORM, tools like this are just essential (we use NH Prof extensively for NHibernate).
A quick test with the original stored procedure showed a query execution time of 24ms. Swapping this out with the PagedList implementation brought it down to <1ms - the SQL generated:
SELECT TOP ( 10 ) [Project1].[ProductId] AS [ProductId],
[Project1].[Name] AS [Name],
[Project1].[ShortDescription] AS [ShortDescription],
[Project1].[FullDescription] AS [FullDescription],
[Project1].[AdminComment] AS [AdminComment],
[Project1].[TemplateID] AS [TemplateID],
[Project1].[ShowOnHomePage] AS [ShowOnHomePage],
[Project1].[MetaKeywords] AS [MetaKeywords],
[Project1].[MetaDescription] AS [MetaDescription],
[Project1].[MetaTitle] AS [MetaTitle],
[Project1].[SEName] AS [SEName],
[Project1].[AllowCustomerReviews] AS [AllowCustomerReviews],
[Project1].[AllowCustomerRatings] AS [AllowCustomerRatings],
[Project1].[RatingSum] AS [RatingSum],
[Project1].[TotalRatingVotes] AS [TotalRatingVotes],
[Project1].[Published] AS [Published],
[Project1].[Deleted] AS [Deleted],
[Project1].[CreatedOn] AS [CreatedOn],
[Project1].[UpdatedOn] AS [UpdatedOn]
FROM (SELECT [Project1].[ProductId] AS [ProductId],
[Project1].[Name] AS [Name],
[Project1].[ShortDescription] AS [ShortDescription],
[Project1].[FullDescription] AS [FullDescription],
[Project1].[AdminComment] AS [AdminComment],
[Project1].[TemplateID] AS [TemplateID],
[Project1].[ShowOnHomePage] AS [ShowOnHomePage],
[Project1].[MetaKeywords] AS [MetaKeywords],
[Project1].[MetaDescription] AS [MetaDescription],
[Project1].[MetaTitle] AS [MetaTitle],
[Project1].[SEName] AS [SEName],
[Project1].[AllowCustomerReviews] AS [AllowCustomerReviews],
[Project1].[AllowCustomerRatings] AS [AllowCustomerRatings],
[Project1].[RatingSum] AS [RatingSum],
[Project1].[TotalRatingVotes] AS [TotalRatingVotes],
[Project1].[Published] AS [Published],
[Project1].[Deleted] AS [Deleted],
[Project1].[CreatedOn] AS [CreatedOn],
[Project1].[UpdatedOn] AS [UpdatedOn],
row_number()
OVER(ORDER BY [Project1].[Name] ASC) AS [row_number]
FROM (SELECT [Extent1].[ProductId] AS [ProductId],
[Extent1].[Name] AS [Name],
[Extent1].[ShortDescription] AS [ShortDescription],
[Extent1].[FullDescription] AS [FullDescription],
[Extent1].[AdminComment] AS [AdminComment],
[Extent1].[TemplateID] AS [TemplateID],
[Extent1].[ShowOnHomePage] AS [ShowOnHomePage],
[Extent1].[MetaKeywords] AS [MetaKeywords],
[Extent1].[MetaDescription] AS [MetaDescription],
[Extent1].[MetaTitle] AS [MetaTitle],
[Extent1].[SEName] AS [SEName],
[Extent1].[AllowCustomerReviews] AS [AllowCustomerReviews],
[Extent1].[AllowCustomerRatings] AS [AllowCustomerRatings],
[Extent1].[RatingSum] AS [RatingSum],
[Extent1].[TotalRatingVotes] AS [TotalRatingVotes],
[Extent1].[Published] AS [Published],
[Extent1].[Deleted] AS [Deleted],
[Extent1].[CreatedOn] AS [CreatedOn],
[Extent1].[UpdatedOn] AS [UpdatedOn]
FROM [dbo].[Nop_Product] AS [Extent1]
INNER JOIN [dbo].[Nop_Product_Category_Mapping] AS [Extent2]
ON [Extent1].[ProductId] = [Extent2].[ProductID]
WHERE [Extent2].[CategoryID] = 29 /* @p__linq__0 */) AS [Project1]) AS [Project1]
WHERE [Project1].[row_number] > 0
ORDER BY [Project1].[Name] ASC
As I said the main advantage of using a tool like EF Prof is that you can then see exactly what SQL is being generated so you can then further tune your object queries.
Deval,
Good suggestion. Shame really that everything is static. We would probably need to have a static ICache property on each Manager that news up the appropriate ICache implementation
e.g.
public static ICache Cache
{
return new NopMemoryCache();
}
Then we can just call
if (this.Cache.Contains(key))
var products = this.Cache.Get<IList<Product>>(key);
If I get time this week I will have a go at testing this out.
