I saw this when I optimized the SP on your database - yes it looks quite ugly, but my initial impression is that you might have some very deep metaclass hierarchy...
The deep of the metaclass hierarchy isn't the big contributor here, only the system metaclasses generates 15 unnecessary queries per load.
The failure to load all payments can in my opinion occur even without any custom metaclasses at all.
Reading the code more carefully now I think I understand the problem. I will look into it to see if I can come up with something better ...
CREATE TABLE #OrderSearchResults (OrderGroupId int) insert into #OrderSearchResults (OrderGroupId) select OrderGroupId from @results SET @search_condition = N'''INNER JOIN OrderFormPayment O ON O.PaymentId = T.ObjectId INNER JOIN #OrderSearchResults R ON O.OrderGroupId = R.OrderGroupId ''' DECLARE @parentmetaclassid int DECLARE @rowNum int DECLARE @maxrows int DECLARE @tablename nvarchar(120) DECLARE @procedurefull nvarchar(max) SET @parentmetaclassid = (SELECT MetaClassId from [MetaClass] WHERE Name = N'orderformpayment' and TableName = N'orderformpayment') DECLARE @PaymentClasses TABLE ( TableName nvarchar(120), RowIndex int ) INSERT INTO @PaymentClasses SELECT TableName, ROW_NUMBER() OVER (ORDER BY MetaClassId) FROM [MetaClass] WHERE ParentClassId = @parentmetaclassid SET @rowNum = 1 SET @maxrows = (SELECT COUNT(RowIndex) FROM @PaymentClasses) WHILE @rowNum < @maxrows BEGIN SELECT @tablename = TableName FROM @PaymentClasses WHERE RowIndex = @rowNum SET @procedurefull = N'mdpsp_avto_' + @tablename + N'_Search NULL, ' + N'''''''' + @tablename + N''''''+ ' TableName, [O].*'' ,' + @search_condition EXEC (@procedurefull) SET @rowNum = @rowNum + 1 END
This is what it looks like after my first attempt. Seems to work. There is currently a need to load from every metaclasses.
Again, untested by QAs - so use it for testing only. Provide 2x performance gain when I test on your database :)
Our performance test shows quite significant results - (and should be even more for the customers because you should have more metaclasses) - which makes a very good argument for me to integrate it into our core framework :).
Thanks for bringing this into our attention.
Got 12% less reads on ecf_Search_ShoppingCart_CustomerAndName with this modified ecf_Search_OrderGroup, CPU more or less the same. I assume you got 2x on the ecf_Search_OrderGroup or even just this modified part of it :) But still in the right direction, it had a lot of reads before and is run a lot, every % is a win!
Black friday performance hype!
IO is just one part of the measurement - try to turn on time statistics and see ;)
Our test shows 30% improvement in loading orders using APIs. That would be even more - I'm very positive that 40% improvement is possible - for a real site.
It was a test on base branch which screwed up. Darn it
Fair enough, I was profiling the database rather than running with statistics. But there I do see some more % on some fields :) Well it's good news regardless, we'll wait excitedly for the QA team.
Thank you for looking into this issue for us.
We have discovered a problem with your new solution though:
It fails to select the last payment class.
Consider the edge case with only a single payment class - the while loop will never run.
We have new version - which is likely the released one - note that a = was added ;)
DECLARE @search_condition nvarchar(max) -- Return Order Form Payment Collection CREATE TABLE #OrderSearchResults (PaymentId int) insert into #OrderSearchResults (PaymentId) select PaymentId from OrderFormPayment P INNER JOIN @results R ON R.OrderGroupId = P.OrderGroupId SET @search_condition = N'''INNER JOIN OrderFormPayment O ON O.PaymentId = T.ObjectId INNER JOIN #OrderSearchResults R ON O.PaymentId = R.PaymentId ''' DECLARE @parentmetaclassid int DECLARE @rowNum int DECLARE @maxrows int DECLARE @tablename nvarchar(120) DECLARE @procedurefull nvarchar(max) SET @parentmetaclassid = (SELECT MetaClassId from [MetaClass] WHERE Name = N'orderformpayment' and TableName = N'orderformpayment') DECLARE @PaymentClasses TABLE ( query nvarchar(max), RowIndex int ) INSERT INTO @PaymentClasses SELECT query = N'mdpsp_avto_' + tablename + N'_Search NULL, ' + N'''''''' + tablename + N''''''+ ' TableName, [O].*'' ,' + @search_condition, ROW_NUMBER() OVER (ORDER BY MetaClassId) FROM [MetaClass] WHERE ParentClassId = @parentmetaclassid SET @rowNum = 1 SET @maxrows = (SELECT COUNT(RowIndex) FROM @PaymentClasses) WHILE @rowNum <= @maxrows BEGIN SELECT @procedurefull = query FROM @PaymentClasses WHERE RowIndex = @rowNum EXEC (@procedurefull) SET @rowNum = @rowNum + 1 END DROP TABLE #OrderSearchResults
Great, do you know in which version it will be fixed in?
Did you have to buy the QA team beer?
This is not yet merged to develop, so I would guess 11.3 or 11.3.1.
And no, I didn't. QAs still haven't known about this yet.
In one of our sites that runs Commerce v11.2.0 we have some performance issues related to carts.
We generate a lot of calls to the stored procedure "ecf_Search_ShoppingCart_CustomerAndName", partially because we use it to load not only the cart but also for the customer's wishlist.
However, that procedure does in turn call another procedure "ecf_Search_OrderGroup" and this is doing some truly horrifying things:
This iterates over all meta classes in commerce and loads them one by one in a search for concrete implementations of a payment class, if it finds one it does another procedure call to load its posts(if any). For our part this is no less than over 35 calls to dbo.MetaClass, 21 of them are system ones. Each call to the MetaClass table is quite cheap but there is a lot of them and they aggregate up to some really large amounts.
I mean adding two where conditions on ParentClassId=@parentmetaclassid instead of having the check in an if-statement would save 25 calls for each cart/wishlist loaded.
Furthermore I am far from convinced that this code won't result in some really strange behaviors in rare occurences. Normally sql server does indeed return a table in the order it is stored in the database, which is sorted by id when an identifier is used, but without an order by clause I wouldn't assume that it is guaranteed. Taking top 1 as you iterate and checking that the id is greater than the current row's id makes me rather nervous that there is a rare chance it will miss payments when it loads ordergroups, since the name of this procedure can't rule out that it is being called for purchaseorders as well as carts the actual implications for logic processing these incompletly loaded order groups is staggering.
I could fix this myself, but as it goes against all recommendations and guidelines to alter procedures yourselves, I would loathe to do our own implementation of this.
As a side note I would like to see versions of these procecedures as well as the API that allows us to filter by market as well as customerId and name, doing so in code after everything has been loaded feels like a performance thief.
Can someone on Episerver please patch this up as soon as possible?