May 14, 2009 at 2:41 pm
I was asked by a developer to review a segment of code and why it is performing bad in Prod vs. Test
The code is written in a way that I have never seen before...
The jist is as follows
1. First...a simple derived table
- 2 tables joined with a LEFT OUTER JOIN and fairly basic Filters in the WHERE
2. The Derived table is joined to a 2 different UDF that takes in 5 parameters each
- each UDF is joined 3 more times (for a total of 4 LEFT OUTER JOIN's) to the Derived table
3. Each Join to the 2 different UDF's have different criteria as the input variables
4. It is executed as Dynamic Code with the sp_executesql system procedure.
In Test it runs on a single CPU SQL 2000 server in roughly 10 seconds...
In Production it runs in 10 minutes on a SQL 2000 Enterprise Server with 8 CPU's and 12 GB's of RAM
I have tried to review the execution plan which doesn't lead me very far as nothing is really present in it...The execution plan basically shows the first join of the derived table and then the LEFT OUTER JOIN's back to the derived table of the UDF 'Tables'
Each one of the LEFT OUTER JOIN's result in 0% of the execution cost...except the last LEFT OUTER JOIN which accounts for 90% of the execution...
I have tried to run Profiler to see if I can find something...and I get a TON of
Scan:Started
Scan:Stopped
The object ID's where the scan started and stopped are from the UDF tables...
That is about all that I can get out of Profiler
I wanted a logical reply to this developer who wrote this but I don't know how to evaluate this code to come up with a good response
I haven't had to evaluate code with UDF's used in the JOIN criteria before
I have read briefly on Google searches today that UDF's used in this fashion result in a Cursor like behavior Row by Row vs. set based operations...
Can anyone please help me formulate a way to respond or a way to start evaluating the process correctly?
I would greatly appreciate it...
For reference I will post a segment of the code after I reformatted it for easier reading...I hope it keeps the formatting when I post it...if not then i will probably remove it...
ALTER PROCEDURE bp_wr_salesByProductCategory
(
@Year varchar(4),
@Tech_Spec varchar(15)=null,
@Product_Category varchar(50)=null,
@SiteKey varchar(30) = null,
@EmpID varchar(15) = null,
@EliminationFlag varchar(1)=null,
@IncludeNonCommissionedSales int=0
) AS
declare @AcctPeriod Datetime
declare @AcctPeriod_Start Datetime
declare @AcctPeriod_YearEnd DateTime
declare @PreviousYear as varchar(4)
declare @AcctPeriod_LastYear Datetime
declare @AcctPeriod_LastYear_Start DateTime
declare @AcctPeriod_LastYear_YearEnd varchar(10)
declare @ReportAsOfDate Datetime
declare @YTDThroughDate as DateTime
declare @EliminationFilter as varchar(1)
declare @nonCommissonFilter bit
declare @SQL as nvarchar(4000)
declare @ParamDef as nvarchar(1000)
SET @nonCommissonFilter= 0
IF @IncludeNonCommissionedSales<> 0
BEGIN
SET @nonCommissonFilter = null
END
SET @EliminationFilter=null
IF @SiteKey = 'mgmt'
BEGIN
SET @EmpID = null
END
if @EliminationFlag= 'Y'
BEGIN
SET @EliminationFilter='N'
END
IF datepart(year,getdate())=cast(@Year as int)
BEGIN
SET @ReportAsOfDate= getdate()
SET @AcctPeriod= dateadd(m, datediff(m, 0, @ReportAsOfDate)-1 , 0)
END
ELSE
BEGIN
SET @ReportAsOfDate= '12/31/' + @year
SET @AcctPeriod= dateadd(m, datediff(m, 0, @ReportAsOfDate) , 0)
END
SET @PreviousYear= datepart(year,@ReportAsOfDate) -1
SET @AcctPeriod_LastYear= dateadd(year,-1,@AcctPeriod)
SET @AcctPeriod_Start= '1/1/' + cast(datepart(year,@AcctPeriod) as varchar(4))
SET @AcctPeriod_YearEnd= '12/1/' + cast(datepart(year,@AcctPeriod) as varchar(4))
SET @AcctPeriod_LastYear_YearEnd= '12/1/' + cast((datepart(year,@AcctPeriod) - 1) as varchar(4))
SET @AcctPeriod_LastYear_Start= '1/1/' + cast(datepart(year,@AcctPeriod_LastYear) as varchar(4))
SET @YTDThroughDate= dateadd(m, datediff(m, 0, dateadd(m, 1, @AcctPeriod)), -1)
select * from master..sysdatabases where dbid IN (2, 58)
--select @AcctPeriod_Start as AcctPeriod_Start,@AcctPeriod as AcctPeriod,@AcctPeriod_LastYear as AcctPeriod_LastYear,@AcctPeriod_LastYear_Start as AcctPeriod_LastYear_Start,@AcctPeriod_LastYear_YearEnd as AcctPeriod_LastYear_YearEnd,@EmpID as EmpID, @EliminationFilter as EliminationFilter, @nonCommissonFilter as nonCommissonFilter
set @SQL='
SELECT @YTDThroughDate as YTDThroughDate,
Product_Category,
SalesCategory.Tech_Spec,
Plan_YTD.Amount AS Plan_YTD,
Plan_CurrentYear.Amount AS Plan_CurrentYear,
Sales_YTD.Amount AS Sales_YTD,
Sales_CurrentYear.Amount AS Sales_CurrentYear,
Sales_LastYear.Amount AS Sales_LastYear,
Sales_YTD_LastYear.Amount as Sales_YTD_LastYear,
coalesce(Sales_YTD.Amount,0) - coalesce(Plan_YTD.Amount,0) as SalesYTD_vs_PlanYTD,
coalesce(Sales_YTD.Amount,0) - coalesce(Sales_YTD_LastYear.Amount,0) as Growth,
case when isnull(Sales_YTD_LastYear.Amount,0) <> 0
then (coalesce(Sales_YTD.Amount,0) - Sales_YTD_LastYear.Amount)/ Sales_YTD_LastYear.Amount
else 0END as Growth_Percent,
(ISNULL(Forecast_CurrentYear.Amount,0) - ISNULL(Forecast_YTD.Amount,0) + ISNULL(Sales_YTD.Amount,0)) as YEP
FROM (SELECT ISNULL(SMSSelectorGuide.[Product Category (Legacy)], ''unknown'') AS Product_Category,
isnull(tb_sales.Tech_Spec,''unknown'') as Tech_Spec
FROM tb_Sales with (nolock)
LEFT OUTER JOINSMSSelectorGuide with (nolock) ON tb_Sales.Tech_Spec = SMSSelectorGuide.[Internal Spec Number with Routing (SMS)]
WHERE AcctPeriod BETWEEN @AcctPeriod_LastYear_Start AND
@AcctPeriod_YearEndAND
(
@Tech_Spec is null OR
@Tech_Spec = Tech_Spec
)AND
(
@Product_Category is NULL OR
@Product_Category = SMSSelectorGuide.[Product Category (Legacy)]
)AND
(
@nonCommissonFilter is null OR
nonCommissioned= @nonCommissonFilter
)
GROUP BY
ISNULL(SMSSelectorGuide.[Product Category (Legacy)], ''unknown''),
tb_Sales.Tech_Spec
) SalesCategory
LEFT OUTER JOIN(@AcctPeriod_LastYear_Start, @AcctPeriod_LastYear_YearEnd, @EmpID, @EliminationFilter, @nonCommissonFilter) Sales_LastYear ON SalesCategory.Tech_Spec = Sales_LastYear.Tech_Spec
LEFT OUTER JOIN SalesBySpecDateRange(@AcctPeriod_Start,@AcctPeriod_YearEnd,@EmpID,@EliminationFilter,@nonCommissonFilter) Sales_CurrentYear ON SalesCategory.Tech_Spec = Sales_CurrentYear.Tech_Spec
LEFT OUTER JOINPlanBySpecDateRange (@AcctPeriod_Start,@AcctPeriod_YearEnd,''O'',@EmpID,@EliminationFilter) Plan_CurrentYear ON SalesCategory.Tech_Spec = Plan_CurrentYear.SpecNo
LEFT OUTER JOIN PlanBySpecDateRange (@AcctPeriod_Start,@AcctPeriod,''O'',@EmpID,@EliminationFilter) Plan_YTD ON SalesCategory.Tech_Spec = Plan_YTD.SpecNo
LEFT OUTER JOINPlanBySpecDateRange (@AcctPeriod_Start,@AcctPeriod_YearEnd,''R'',@EmpID,@EliminationFilter) Forecast_CurrentYear ON SalesCategory.Tech_Spec = Forecast_CurrentYear.SpecNo
LEFT OUTER JOINPlanBySpecDateRange (@AcctPeriod_Start,@AcctPeriod,''R'',@EmpID,@EliminationFilter) Forecast_YTD ON SalesCategory.Tech_Spec = Forecast_YTD.SpecNo
LEFT OUTER JOINSalesBySpecDateRange(@AcctPeriod_Start,@AcctPeriod,@EmpID,@EliminationFilter,@nonCommissonFilter) Sales_YTD ON SalesCategory.Tech_Spec = Sales_YTD.Tech_Spec
LEFT OUTER JOINSalesBySpecDateRange(@AcctPeriod_LastYear_Start,@AcctPeriod_LastYear,@EmpID,@EliminationFilter,@nonCommissonFilter) Sales_YTD_LastYear ON SalesCategory.Tech_Spec = Sales_YTD_LastYear.Tech_Spec
ORDER BY
SalesCategory.Product_Category, SalesCategory.Tech_Spec'
SET @ParamDef = '@YTDThroughDate as DateTime, @Tech_Spec varchar(15), @Product_Category varchar(50), @nonCommissonFilter bit, @EmpID varchar(15), @EliminationFilter varchar(1)'
SET @ParamDef = @ParamDef + ', @AcctPeriod_LastYear DateTime, @AcctPeriod_LastYear_YearEnd DateTime, @AcctPeriod_Start DateTime, @AcctPeriod_YearEnd DateTime'
SET @ParamDef = @ParamDef + ', @AcctPeriod DateTime, @AcctPeriod_LastYear_Start DateTime'
--SELECT @ParamDef as Params
EXECUTE sp_executesql @SQL, @ParamDef,
@YTDThroughDate=@YTDThroughDate,
@Tech_Spec=@Tech_Spec,
@Product_Category=@Product_Category,
@nonCommissonFilter=@nonCommissonFilter,
@EmpID=@EmpID,
@EliminationFilter=@EliminationFilter,
@AcctPeriod_LastYear=@AcctPeriod_LastYear,
@AcctPeriod_LastYear_YearEnd=@AcctPeriod_LastYear_YearEnd,
@AcctPeriod_Start=@AcctPeriod_Start,
@AcctPeriod_YearEnd=@AcctPeriod_YearEnd,
@AcctPeriod=@AcctPeriod,
@AcctPeriod_LastYear_Start=@AcctPeriod_LastYear_Start
May 14, 2009 at 9:46 pm
Well one thing that jumped out at me right away was this:
Lee Hart (5/14/2009)
...'(
@Tech_Spec is null OR
@Tech_Spec = Tech_Spec
)AND
(
@Product_Category is NULL OR
@Product_Category = SMSSelectorGuide.[Product Category (Legacy)]
)AND
(
@nonCommissonFilter is null OR
nonCommissioned= @nonCommissonFilter
)
'...
These so called "Catch All' clauses are killers. As long as you are doing dynamic SQL anyway, you should you should be using conditionals to test these parameters for NULL before hand and just not include them if they are, otherwise you can make them a simple equality test:
' ... '
+ CASE When @Tech_Spec IS NOT null
THEN 'AND @Tech_Spec = Tech_Spec ' ELSE '' END
+ CASE When @Product_Category IS NOT null
THEN 'AND @Product_Category = SMSSelectorGuide.[Product Category (Legacy)] ' ELSE '' END
+ CASE When @nonCommissonFilter IS NOT null
then 'AND @nonCommissonFilter = nonCommissioned ' ELSE '' END
+ ' ... '
(Edit: FYI, added ELSEs to my CASEs to be correct.)
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
May 15, 2009 at 7:55 am
Hi Barry...
Let me start by saying thanks for looking at this question and giving a great answer...I didn't even catch that portion of the code...I was fixated on the UDF's for the tables that were joining to the Derived Table...
I read a decent article explaining what you referred to that made a lot more sense afterwards http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/
The part of this issue that still doesn't make a whole lot of sense to me is why it would perform in under 15 seconds on a slow test server (same OS and SP level) as a production server with 8 CPU's and 12 GB's of RAM...
I don't know of the correct way of monitoring the code either with Profiler or the execution plan...I am not getting anything tangible from either to point to one specific thing...
This whole process is still in development so the database that it is running on is very small. The biggest table in play here only has 100,000 rows and the rest have under 20,000
I was thinking it had something to do with the number of CPU's 1 vs. 8 and some level of parallel execution but I don't see it either from sp_who2 or through Profiler...
Thanks again!
Lee
May 15, 2009 at 8:49 am
Lee Hart (5/15/2009)
The part of this issue that still doesn't make a whole lot of sense to me is why it would perform in under 15 seconds on a slow test server (same OS and SP level) as a production server with 8 CPU's and 12 GB's of RAM...
In all likelihood, it is using a different plan or the table(s) are significantly different.
I don't know of the correct way of monitoring the code either with Profiler or the execution plan...I am not getting anything tangible from either to point to one specific thing...
Hmm, I'm not sure what the settings are in SQL 2000. Maybe someone else does (Gail?, Grant?). What you should be able to do is to put the exact command into you query window on your prodction server and then display the query plan. If you compare that to the query plan from your test server, you should see some differences.
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
May 15, 2009 at 10:32 am
OK so I took a step back and decieded to re-review everything involved and decided to do the obvious and evaluate the tables with a DBCC showcontig and get an idea of the status of everything in play...
Even though the tables were fairly small (biggest had 101,000 rows) the scan density was terrible.
I re-indexed the database and set up a maintenance plan for it (previously didn't have anything as it is still in development)
once finished I re-ran the code and it completed in 10 seconds...pretty good compared to the 7 to 10 minutes it ran previously.
The execution plan showed the use of Parallelism / Repartition Streams...where it didn't before.
I am still going to make recommendations to the developer that you pointed out to see if even further enhancements can be made.
Sometimes the easiest answer is the best.
Thanks Barry again for reply...it did help me along the way and I learned something!
Lee
May 15, 2009 at 11:28 am
Berry had some good suggestions. Those "catch-alls" can be nasty. I just did a quick glance and didn't see a real reason why this is all dynamic. I'm sure there is a good reason why the developers are using so many UDFs in joins, but Seems like poor coding to me. What is the performance differential if you eliminate those UDFs (i.e. move the code out of the UDF into the query)? Can you create temp tables or table variables (with PK or proper indexing) load those from the UDF and replace the UDF joins with those temp tables? Sometimes you'll get good gains by doing this because SQL doesn't seem to handle UDF joins very well.
Cheers!
May 15, 2009 at 11:37 am
Lamprey13 (5/15/2009)
Berry had some good suggestions. Those "catch-alls" can be nasty. I just did a quick glance and didn't see a real reason why this is all dynamic. I'm sure there is a good reason why the developers are using so many UDFs in joins, but Seems like poor coding to me. What is the performance differential if you eliminate those UDFs (i.e. move the code out of the UDF into the query)? Can you create temp tables or table variables (with PK or proper indexing) load those from the UDF and replace the UDF joins with those temp tables? Sometimes you'll get good gains by doing this because SQL doesn't seem to handle UDF joins very well.Cheers!
What you asked is exactly what I recommended to the developer who wrote this code. if he can build the set of temp tables based on that criteria being sent to the UDF and then join on a temp table.
I also used Barry example for the Catch all WHERE clause and the recommended changes...
I have made similar requests to the developer and his manager in the past when code similar to this was asked to be reviewed...it fell on deaf ears unfortunately...
Viewing 7 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply