AI OnAI Off

Unnecessary performance hits when loading carts/wishlists

Erik Norberg
Erik Norberg
Vote:
 

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:

-- Return Order Form Payment Collection
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 @metaclassid int
DECLARE @parentclassid int
DECLARE @parentmetaclassid int
DECLARE @rowNum int
DECLARE @maxrows int
DECLARE @tablename nvarchar(120)
DECLARE @name nvarchar(120)
DECLARE @procedurefull nvarchar(max)
SET @parentmetaclassid = (SELECT MetaClassId from [MetaClass] WHERE Name = N'orderformpayment' and TableName = N'orderformpayment')
SELECT top 1 @metaclassid = MetaClassId, @tablename = TableName, @parentclassid = ParentClassId, @name = Name from [MetaClass]
	SELECT @maxRows = count(*) from [MetaClass]
	SET @rowNum = 0
	WHILE @rowNum < @maxRows
	BEGIN
		SET @rowNum = @rowNum + 1
		IF (@parentclassid = @parentmetaclassid)
		BEGIN
			SET @procedurefull = N'mdpsp_avto_' + @tablename + N'_Search NULL, ' + N'''''''' + @tablename + N''''''+  ' TableName, [O].*'' ,'  + @search_condition
			EXEC (@procedurefull)
		END
		SELECT top 1 @metaclassid = MetaClassId, @tablename = TableName, @parentclassid = ParentClassId, @name = Name from [MetaClass] where MetaClassId > @metaclassid
	END
DROP TABLE #OrderSearchResults

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? cry

#182968
Oct 02, 2017 18:04
Quan Mai
Quan Mai
Vote:
 

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...

#182982
Oct 03, 2017 0:02
Erik Norberg
Erik Norberg
Vote:
 

Hello Quan

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.

/EN

#183003
Oct 03, 2017 11:24
Quan Mai
Quan Mai
Vote:
 

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 ... 

#183034
Oct 03, 2017 16:18
Quan Mai
Quan Mai
Vote:
  
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 :)

#183041
Oct 03, 2017 17:33
Quan Mai
Quan Mai
Vote:
 

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. 

#183042
Oct 03, 2017 19:45
Joel Yourstone
Joel Yourstone
Vote:
 

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! cool

#183043
Oct 03, 2017 19:55
Quan Mai
Quan Mai
Vote:
 

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 

#183044
Edited, Oct 03, 2017 20:12
Joel Yourstone
Joel Yourstone
Vote:
 

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.

#183045
Oct 03, 2017 20:27
Erik Norberg
Erik Norberg
Vote:
 

Hello Quan

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.

/EN

#183102
Oct 05, 2017 12:16
Quan Mai
Quan Mai
Vote:
 

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
#183106
Oct 05, 2017 12:32
Erik Norberg
Erik Norberg
Vote:
 

Great, do you know in which version it will be fixed in?

Did you have to buy the QA team beer? cool

#183117
Oct 05, 2017 14:15
Quan Mai
Quan Mai
Vote:
 

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.

#183120
Oct 05, 2017 14:22
Bob Bolt
Bob Bolt
Vote:
 

COM 5594 was fixed in this week's release, 11.4

#185291
Nov 14, 2017 16:19
This topic was created over six months ago and has been resolved. If you have a similar question, please create a new topic and refer to this one.
* You are NOT allowed to include any hyperlinks in the post because your account hasn't associated to your company. User profile should be updated.