November 23, 2010 at 3:43 am
With the use of # tables like you have in these procedures, there are no indexes, so when you do the final SELECT, you will be doing a full table scan not efficient.
There is also a lot of waste:
SELECT * INTO
No objects qualified
Function calls to look up products
Lots to be gained, but quite honestly, I think its time to call a professional...
November 23, 2010 at 4:13 am
Some other potential optimizations. You have a whole bunch of DISTINCT operations. These are aggregates and when you see them everywhere in a query it's usually indicative of poor programming ("we always use them" or improper joins, others) or bad structures in the database. You could attack that right away, remove them where they are inappropriate, and see possible improvements. Also, right out of the gate you're loading a bunch of rows into a table variable, @interactions, and the referring to values within that table variable in operations such as
not in (select ID from @interactions)
Table variables don't have statistics. As such, this type of interaction can only be satisfied by a scan. At the very least, I'd suggest swapping this out with a temporary table, but I'd also reexamine that whole mechanism or loading data into one table in order to load it into another in order to eliminate data from the load, etc. That's frequently a performance issue.
There's lots more. You've got correlated sub-queries in the SELECT statement such as this one:
(select count(*) from dbo.interaction_product_indication_core_messages where interaction_product_indication_id in (select id from dbo.interaction_product_indications where interaction_id = #interactions.id)) as CoreMessagesCount
Again, these can lead to poor performance and frequently can be added to other pieces of the WHERE clause to arrive at better queries.
Your developer is flat out wrong. There are tons and tons of optimizations available to this query. None of us has even seen an execution plan yet. Fix all the suggestions from everyone else, then generate an execution plan, and I'll be we can keep going.
The fact is, you can almost always find more optimizations if you keep looking. There is a point where you begin to see diminishing returns, where the amount of work you put in for the amount of optimization you get out makes it not worth it. You're nowhere near that point yet.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
November 23, 2010 at 5:26 am
Gangadhara MS (11/21/2010)
I have followed the same steps and found that some stored procedures having highest duration abnd IO/CPU.But my developer saying i can't further tune with query as this is the complex procedure.
Here's a thought no one has thought to mention yet.
You need to investigate WHY your developer is telling you this. Is he just being lazy? Or is this a third-party vendor stored procedure that he literally doesn't have permissions to alter?
If the later, then it's time to get the vendor involved in this process. If the former, you need to go to the developer's boss and explain to the boss that you need the developer to rewrite this query and show him why. You might also want to get your boss involved.
November 23, 2010 at 5:36 am
Brandie Tarvin (11/23/2010)
Gangadhara MS (11/21/2010)
I have followed the same steps and found that some stored procedures having highest duration abnd IO/CPU.But my developer saying i can't further tune with query as this is the complex procedure.
Here's a thought no one has thought to mention yet.
You need to investigate WHY your developer is telling you this. Is he just being lazy? Or is this a third-party vendor stored procedure that he literally doesn't have permissions to alter?
If the later, then it's time to get the vendor involved in this process. If the former, you need to go to the developer's boss and explain to the boss that you need the developer to rewrite this query and show him why. You might also want to get your boss involved.
I completely agree with what you are saying.
Now without trying sound like a right P**** but if the question of "what next" is being asked here (no execution plan test, nothing) and he believes a developer that is saying it cant be tuned (especially that mess), I dont think he has the knowledge or business presence to do that - hence saying get a professional.
Though if it is some that is 3rd party, definitely hand it back saying "fix".
November 23, 2010 at 5:41 am
A few more comments on your code. Look at your first three insert statements. Remove the subqueries, do LEFT OUTER JOINS with the subqueries, and look at making all three insert statements one insert instead.
Example (this needs tested!!! And make sure you understand why I did the things I did before you implement this code!!!):
insert into @interactions(ID,interaction_date,note,interaction_type_id,interaction_location_id,interaction_status_id,employee_id)
select distinct interactions.ID,interaction_date,note,interaction_type_id,interaction_location_id,interaction_status_id,interactions.employee_id
from attendees WITH (NOLOCK)
inner join interaction_attendees WITH (NOLOCK)
on attendees.id = interaction_attendees.attendee_id
inner join interactions WITH (NOLOCK)
on interaction_attendees.interaction_id=interactions.id
LEFT OUTER JOIN interaction_types WITH (NOLOCK)
on interactions.interaction_type_id=interaction_types.id
LEFT OUTER JOIN hcp_account hcp WITH (NOLOCK)
ON xxx.hcp_id /*needs table alias in place of xxx*/ = hcp.hcp_id
AND (hcp.Account_ID = @account_id OR Department_ID=@account_id)
LEFT OUTER JOIN locations WITH (NOLOCK)
ON interactions.interaction_location_ID=locations.id
LEFT OUTER JOIN @interactions ia1
ON locations.ID = ia1.ID
where (attendees.account_id=@account_id and interaction_Status_id=5 and is_event=0 and interactions_types.ID IS NOT NULL) --remove the sponsor account for the events
OR (hcp.hcp_id IS NOT NULL and interaction_Status_id=5) --Second insert statement WHERE clause redone
OR (locations.account_ID=@account_id and interaction_Status_id=5 and ia1.ID IS NULL) --Third insert statement WHERE clause redone
Ditto on all of Dave's statements. Look over your code and optimize as he suggested.
Don't use dynamic SQL in your second proc. There's no need for it. Use a Try...Catch block around your IF/THEN statement. The Catch will catch any errors and enable you to debug and you can just run the code if there are no errors.
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).
November 23, 2010 at 5:47 am
...
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).
thats my understanding too
November 23, 2010 at 5:50 am
grahamc (11/23/2010)
hence saying get a professional.
Agreed. Often just a few days from someone who really knows their way around tuning procs can make all the difference. Plus there's the opportunity to learn from them
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
November 23, 2010 at 5:52 am
grahamc (11/23/2010)
...
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).
thats my understanding too
It does cache the plan for re-use but there if the sarg's are hardcoded into the statement , then it is a different statement
ie
Select * from table where col = 'value'
is a different statement to
Select * from table where col = 'a different value'
where as
Declare @Var varchar(XX)
Select @Var = 'value'
Select * from table where col = @Var
and
Declare @Var varchar(XX)
Select @Var = 'a different value'
Select * from table where col = @Var
will use the same plans.
EDIT -- Optimize for adhoc and forced parametrization changes things
November 23, 2010 at 5:52 am
Brandie Tarvin (11/23/2010)
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).
You're wrong. 😀
SQL can cache dynamic or ad-hoc SQL just as it does stored procs (you're thinking back SQL 6.5 or so era when it didn't). The main difference is in mapping and reuse of exec plans. With dynamic SQL (or ad-hoc SQL) the match is done on a hash of the query test.
So the plan will be cached. It may not be possible for it to be reused though. Depends...
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
November 23, 2010 at 6:02 am
GilaMonster (11/23/2010)
grahamc (11/23/2010)
hence saying get a professional.Agreed. Often just a few days from someone who really knows their way around tuning procs can make all the difference. Plus there's the opportunity to learn from them
precisely...
GilaMonster (11/23/2010)
Brandie Tarvin (11/23/2010)
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).You're wrong. 😀
SQL can cache dynamic or ad-hoc SQL just as it does stored procs (you're thinking back SQL 6.5 or so era when it didn't). The main difference is in mapping and reuse of exec plans. With dynamic SQL (or ad-hoc SQL) the match is done on a hash of the query test.
So the plan will be cached. It may not be possible for it to be reused though. Depends...
my new piece of knowledge for the day, thanks
November 23, 2010 at 6:02 am
GilaMonster (11/23/2010)
Brandie Tarvin (11/23/2010)
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).You're wrong. 😀
SQL can cache dynamic or ad-hoc SQL just as it does stored procs (you're thinking back SQL 6.5 or so era when it didn't). The main difference is in mapping and reuse of exec plans. With dynamic SQL (or ad-hoc SQL) the match is done on a hash of the query test.
So the plan will be cached. It may not be possible for it to be reused though. Depends...
Just to add real world scenario here. I have a system in prod where 95% of all server use comes from a search form (really complex query... 500 to 1000 lines depending on the scenario and using ± 120 different tables).
I just had a look and there are 8000+ plans cached for that query so I guess plan reuse is minimal at best :w00t: (there are only 4000 - 5000 searches per day on the site).
Also when I look at the compile time of those statements, they require a staggering 67% of the run time of the queries (250 ms out of 375 avg ms).
So out of 33 minutes of daily processing, 22 minutes come from compiling and only 1 hour comes from executing the complex queries.
So yes dynamic sql has an impact of performance. The only question is how much impact on your system. I would assume a lot less if you have a more "normal" search form.
November 23, 2010 at 6:10 am
Ninja's_RGR'us (11/23/2010)
GilaMonster (11/23/2010)
Brandie Tarvin (11/23/2010)
Only ever use Dynamic SQL when you absolutely have to because dynamic SQL can't cache any execution plans for re-use later, which is a performance hit (I hope Gail will correct me if I'm wrong...).You're wrong. 😀
SQL can cache dynamic or ad-hoc SQL just as it does stored procs (you're thinking back SQL 6.5 or so era when it didn't). The main difference is in mapping and reuse of exec plans. With dynamic SQL (or ad-hoc SQL) the match is done on a hash of the query test.
So the plan will be cached. It may not be possible for it to be reused though. Depends...
Just to add real world scenario here. I have a system in prod where 95% of all server use comes from a search form (really complex query... 500 to 1000 lines depending on the scenario and using ± 120 different tables).
I just had a look and there are 8000+ plans cached for that query so I guess plan reuse is minimal at best :w00t: (there are only 4000 - 5000 searches per day on the site).
Also when I look at the compile time of those statements, they require a staggering 67% of the run time of the queries (250 ms out of 375 avg ms).
So out of 33 minutes of daily processing, 22 minutes come from compiling and only 1 hour comes from executing the complex queries.
So yes dynamic sql has an impact of performance. The only question is how much impact on your system. I would assume a lot less if you have a more "normal" search form.
I used to have a search for like that 😛
1 procedure will call up to 15 other procedures, just to build the dynamic SQL statement.... :hehe:
Ended up writing a "quick search" form that had say the top 5 searches (first name, last name, fsa number, fsa name, fsa postcode) with an advanced search button that went to the old screen. These 5 queries accounted for something like 84% of the searches done and were a fair bit faster than the dynamic SQL.
November 23, 2010 at 7:00 am
If I missed this suggestion then my apologies. Besides the obvious optimization suggestions already made, you also need to remove the nolock hint from your insert queries at a minimum. I would remove it altogether, but start with the Insert statements.
NoLock is notoriously bad and can cause dirty reads (multiple reads, skipped reads, duplicated data etc etc etc).
This is probably being used as a bandaid due to poor performance. Making the optimizations already suggested and ensuring appropriate indexes are created will be better for most cases.
Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events
November 23, 2010 at 7:13 am
On the first stored procedure, I'd convert all the inserts into the table variable into a single CTE, with a few OR statements in the Where clause. Then get rid of the temp tables and turn the rest into a single select statement that joins to the CTE. That would take about 15 minutes, and would definitely result in a significant performance improvement.
Second, I'd take a look at the two UDFs in the final update statements (which would be in the Select statement itself, not updates to added columns in a temp table). They can probably be converted to either inline sub-queries in the Select clause, or to a pair of CTEs in the With clause and then joined to in From. If they are irredemably procedural and require scalar UDF format (highly unlikely), I'd leave them in the Select as UDFs, which would be less optimal than the other ideas, but might be necessary.
At first look, I would suspect the two UDF updates are probably the biggest performance-killers in the proc. That's true far too often.
Post the UDF code, if you can, and I may take a real shot at rewriting this so you can test it. I'll be stealing food from some consultant's table if I do, but the challenge looks potentially too fun to pass up.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
November 23, 2010 at 7:18 am
GSquared (11/23/2010)
On the first stored procedure, I'd convert all the inserts into the table variable into a single CTE, with a few OR statements in the Where clause. Then get rid of the temp tables and turn the rest into a single select statement that joins to the CTE. That would take about 15 minutes, and would definitely result in a significant performance improvement.Second, I'd take a look at the two UDFs in the final update statements (which would be in the Select statement itself, not updates to added columns in a temp table). They can probably be converted to either inline sub-queries in the Select clause, or to a pair of CTEs in the With clause and then joined to in From. If they are irredemably procedural and require scalar UDF format (highly unlikely), I'd leave them in the Select as UDFs, which would be less optimal than the other ideas, but might be necessary.
At first look, I would suspect the two UDF updates are probably the biggest performance-killers in the proc. That's true far too often.
Post the UDF code, if you can, and I may take a real shot at rewriting this so you can test it. I'll be stealing food from some consultant's table if I do, but the challenge looks potentially too fun to pass up.
If you are going to post the code as requested by Gus, you should also make sure to post table definitions and some sample data.
Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events
Viewing 15 posts - 16 through 30 (of 32 total)
You must be logged in to reply to this topic. Login to reply