Home » RDBMS Server » Performance Tuning » Optimizing the procedure (10.2.0.4, Solaris 10)
| Optimizing the procedure [message #426986] |
Tue, 20 October 2009 08:40  |
khan500
Messages: 2
Registered: October 2009
|
Junior Member |
|
|
Hello everyone,
Looking for advice as how to optimize a procedure,
I have a procedure
It uses cursor, updates around million records. Takes around 45 Mins to complete. I wanted to use bulk collect and changed the code as given below, to my suprise the execution time remains the same. Any gurus who can give me a hint on what changes needs to be done.
PROCEDURE DRIVE_TEST_BINNING_SINGLE_ZONE(p_bin_size IN NUMBER, p_binning_input_table IN VARCHAR,p_binning_output_table IN VARCHAR, p_srid IN NUMBER) IS
binningFactor NUMBER(5);
binLongitude NUMBER(10,6);
p_latitude NUMBER(10,8);
p_longitude NUMBER(10,8);
binLatitude NUMBER(10,6);
binName VARCHAR(100);
v_utm_point sdo_geometry;
v_point sdo_geometry;
pl_sql_text varchar2(1500):= '';
v_index number(3);
TYPE DT_LAT_LONG_ROW_TYPE IS REF CURSOR;
drive_test_lat_long DT_LAT_LONG_ROW_TYPE;
query_str VARCHAR(1000) := 'SELECT /*+ PARALLEL('|| p_binning_input_table ||', 32) */ longitude,latitude
FROM '||p_binning_input_table||' GROUP BY longitude, latitude'; -- HAVING COUNT(*) > 1';
BEGIN
dbms_output.PUT_LINE('INSIDE SINGLE ZONE');
binningFactor := p_bin_size;
OPEN drive_test_lat_long FOR query_str;
LOOP
FETCH drive_test_lat_long INTO P_LATITUDE, P_LONGITUDE;
v_utm_point := MDSYS.sdo_cs.transform(MDSYS.SDO_GEOMETRY(2001, 8307,MDSYS.sdo_point_type(P_longitude,p_latitude, NULL), NULL, NULL),p_srid);
v_point := MDSYS.sdo_cs.transform(MDSYS.SDO_GEOMETRY(2001, p_srid, MDSYS.sdo_point_type(((FLOOR(v_utm_point.sdo_point.X/binningFactor))*binningFactor)+binningFactor/2, ((CEIL(v_utm_point.sdo_point.Y/binningFactor))*binningFactor)-binningFactor/2, NULL), NULL, NULL),8307);
binLongitude := TRUNC(v_point.sdo_point.X,6);
binLatitude := TRUNC(v_point.sdo_point.Y,6);
binName := 'BIN('||binLongitude||','||binLatitude||','||binningFactor||')';
query_str := 'UPDATE '||p_binning_input_table||' SET BIN_NAME='''||binName||''',BIN_LATITUDE='||binLatitude||',BIN_LONGITUDE='||binLongitude||' WHERE bin_name IS NULL AND latitude = '||P_latitude||' AND longitude='||P_longitude;
EXECUTE IMMEDIATE query_str;
end loop;
IF MOD(drive_test_lat_long%rowcount, 1000) = 0 THEN
COMMIT;
END IF;
EXIT WHEN drive_test_lat_long%NOTFOUND;
END LOOP;
CLOSE drive_test_lat_long;
COMMIT;
END DRIVE_TEST_BINNING_SINGLE_ZONE;
CHANGED CODE WITH BULK COLLECT
PROCEDURE DRIVE_TEST_BINNING_SINGLE_ZONE(p_bin_size IN NUMBER, p_binning_input_table IN VARCHAR,p_binning_output_table IN VARCHAR, p_srid IN NUMBER) IS
binningFactor NUMBER(5);
binLongitude NUMBER(10,6);
binLatitude NUMBER(10,6);
binName VARCHAR(100);
v_utm_point sdo_geometry;
v_point sdo_geometry;
pl_sql_text varchar2(1500):= '';
v_index number(3);
TYPE DT_LAT_LONG_ROW_TYPE IS REF CURSOR;
drive_test_lat_long DT_LAT_LONG_ROW_TYPE;
type latlon is record (longitude dbms_sql.number_table ,latitude dbms_sql.number_table );
lrec latlon;
query_str VARCHAR(1000) := 'SELECT /*+ PARALLEL('|| p_binning_input_table ||', 32) */ longitude,latitude
FROM '||p_binning_input_table||' GROUP BY longitude, latitude'; -- HAVING COUNT(*) > 1';
BEGIN
dbms_output.PUT_LINE('INSIDE SINGLE ZONE');
binningFactor := p_bin_size;
OPEN drive_test_lat_long FOR query_str;
LOOP
FETCH drive_test_lat_long bulk collect INTO lrec.longitude, lrec.latitude limit 300;
for i in 1..lrec.longitude.count
loop
v_utm_point := MDSYS.sdo_cs.transform(MDSYS.SDO_GEOMETRY(2001, 8307,MDSYS.sdo_point_type(lrec.longitude(i), lrec.latitude(i), NULL), NULL, NULL),p_srid);
v_point := MDSYS.sdo_cs.transform(MDSYS.SDO_GEOMETRY(2001, p_srid, MDSYS.sdo_point_type(((FLOOR(v_utm_point.sdo_point.X/binningFactor))*binningFactor)+binningFactor/2, ((CEIL(v_utm_point.sdo_point.Y/binningFactor))*binningFactor)-binningFactor/2, NULL), NULL, NULL),8307);
binLongitude := TRUNC(v_point.sdo_point.X,6);
binLatitude := TRUNC(v_point.sdo_point.Y,6);
binName := 'BIN('||binLongitude||','||binLatitude||','||binningFactor||')';
query_str := 'UPDATE '||p_binning_input_table||' SET BIN_NAME='''||binName||''',BIN_LATITUDE='||binLatitude||',BIN_LONGITUDE='||binLongitude||' WHERE bin_name IS NULL AND latitude = '||lrec.latitude(i)||' AND longitude='||lrec.longitude(i);
EXECUTE IMMEDIATE query_str;
end loop;
IF MOD(drive_test_lat_long%rowcount, 1000) = 0 THEN
COMMIT;
END IF;
EXIT WHEN drive_test_lat_long%NOTFOUND;
END LOOP;
CLOSE drive_test_lat_long;
COMMIT;
END DRIVE_TEST_BINNING_SINGLE_ZONE;
|
|
|
|
| Re: Optimizing the procedure [message #426989 is a reply to message #426986] |
Tue, 20 October 2009 08:48  |
cookiemonster
Messages: 13959
Registered: September 2008
Location: Rainy Manchester
|
Senior Member |
|
|
Can you please report the code formatted and using code tags - see the orafaq forum guide if you're not sure how. That's fairly unreadable.
Can you rewrite it as a single update statement - that'll give the best performance.
|
|
|
|
| Re: Optimizing the procedure [message #426994 is a reply to message #426986] |
Tue, 20 October 2009 09:01 |
cookiemonster
Messages: 13959
Registered: September 2008
Location: Rainy Manchester
|
Senior Member |
|
|
The other thing you really need to do is work out which bit(s) of the procedure are actually slow. Trace the session, use tkprof and analyze the results.
I suspect you've sped up the fast bit.
|
|
|
|
| Re: Optimizing the procedure [message #426995 is a reply to message #426986] |
Tue, 20 October 2009 09:03 |
JRowbottom
Messages: 5933
Registered: June 2006
Location: Sunny North Yorkshire, ho...
|
Senior Member |
|
|
How many different tables do you do this for?
It might well be worth writing a seperate cursor & update for each one to avoid the overheads of dynamic SQL.
|
|
|
|
| Re: Optimizing the procedure [message #426997 is a reply to message #426986] |
Tue, 20 October 2009 09:08 |
JRowbottom
Messages: 5933
Registered: June 2006
Location: Sunny North Yorkshire, ho...
|
Senior Member |
|
|
The execution time remains the same because in 10g Cursor fetches are done as Bulk Collects Size 100 behind the scenes to improve performance.
The only major performance improvement I can see would be rewriting the whole thing as a couple of single queries rather than having the loops in there.
|
|
|
|
|
|
|
|
| Re: Optimizing the procedure [message #427114 is a reply to message #426998] |
Wed, 21 October 2009 03:41 |
JRowbottom
Messages: 5933
Registered: June 2006
Location: Sunny North Yorkshire, ho...
|
Senior Member |
|
|
It may be possible to include those function calls into a singe SQL UPDATE (or MERGE) statement - have you tried this?
I can see that the code runs for a singel table at a time - are you saying that there is only ever one table that is updated using this procedure?
If so, rip all the dynamic SQL (the Execute Immediate & cursor definition) and replace them with normal SQL - that should see some performance improvement.
|
|
|
|
| Re: Optimizing the procedure [message #427782 is a reply to message #426986] |
Sun, 25 October 2009 09:14 |
michael_bialik
Messages: 621
Registered: July 2006
|
Senior Member |
|
|
One problem of many:
You rare not using bind variables, so each update statement must be hard-parsed by Oracle:
query_str := 'UPDATE '||p_binning_input_table||'
SET BIN_NAME='''||binName||''',BIN_LATITUDE='||binLatitude||',
BIN_LONGITUDE='||binLongitude||' WHERE bin_name IS NULL AND
latitude = '||lrec.latitude(i)||' AND longitude='||lrec.longitude(i);
EXECUTE IMMEDIATE query_str;
Try instead:
query_str := 'UPDATE '||p_binning_input_table||' SET BIN_NAME= :binName,
BIN_LATITUDE=:binLatitude,BIN_LONGITUDE=:binLongitude
WHERE bin_name IS NULL AND latitude = :latitude AND longitude= :longitude';
EXECUTE IMMEDIATE query_str
USING binName, binLatitude, binLongitude, lrec.latitude(i),
lrec.longitude(i);
HTH.
P.S. If performance problem still persists - run SQL_TRACE and post TKPROF.
[Updated on: Sun, 25 October 2009 09:43] by Moderator Report message to a moderator |
|
|
|
|
|
| Re: Optimizing the procedure [message #427793 is a reply to message #426986] |
Sun, 25 October 2009 09:58 |
 |
BlackSwan
Messages: 26766
Registered: January 2009
Location: SoCal
|
Senior Member |
|
|
PROCEDURE Drive_test_binning_single_zone
(p_bin_size IN NUMBER,
p_binning_input_table IN VARCHAR,
p_binning_output_table IN VARCHAR,
p_srid IN NUMBER)
IS
binningfactor NUMBER(5);
binlongitude NUMBER(10,6);
binlatitude NUMBER(10,6);
binname VARCHAR(100);
v_utm_point SDO_GEOMETRY;
v_point SDO_GEOMETRY;
pl_sql_text VARCHAR2(1500) := '';
v_index NUMBER(3);
TYPE dt_lat_long_row_type IS REF CURSOR;
drive_test_lat_long DT_LAT_LONG_ROW_TYPE;
TYPE latlon IS RECORD(longitude dbms_sql.number_table,
latitude dbms_sql.number_table);
lrec LATLON;
query_str VARCHAR(1000) := 'SELECT /*+ PARALLEL('
||p_binning_input_table
||', 32) */ longitude,latitude FROM '
||p_binning_input_table
||' GROUP BY longitude, latitude'; -- HAVING COUNT(*) > 1';
BEGIN
dbms_output.Put_line('INSIDE SINGLE ZONE');
binningfactor := p_bin_size;
OPEN drive_test_lat_long FOR query_str;
LOOP
FETCH drive_test_lat_long BULK COLLECT INTO lrec.longitude,lrec.latitude LIMIT 300;
FOR i IN 1.. lrec.longitude.COUNT LOOP
v_utm_point := mdsys.sdo_cs.Transform(mdsys.Sdo_geometry(2001,8307,mdsys.Sdo_point_type(lrec.Longitude(i),lrec.Latitude(i),NULL),
NULL,NULL),p_srid);
v_point := mdsys.sdo_cs.Transform(mdsys.Sdo_geometry(2001,p_srid,mdsys.Sdo_point_type(((Floor(v_utm_point.sdo_point.x / binningfactor)) * binningfactor) + binningfactor / 2,
((Ceil(v_utm_point.sdo_point.y / binningfactor)) * binningfactor) - binningfactor / 2,
NULL),NULL,
NULL),8307);
binlongitude := Trunc(v_point.sdo_point.x,6);
binlatitude := Trunc(v_point.sdo_point.y,6);
binname := 'BIN('
||binlongitude
||','
||binlatitude
||','
||binningfactor
||')';
query_str := 'UPDATE '
||p_binning_input_table
||' SET BIN_NAME='''
||binname
||''',BIN_LATITUDE='
||binlatitude
||',BIN_LONGITUDE='
||binlongitude
||' WHERE bin_name IS NULL AND latitude = '
||lrec.Latitude(i)
||' AND longitude='
||lrec.Longitude(i);
EXECUTE IMMEDIATE query_str;
END LOOP;
IF Mod(drive_test_lat_long%ROWCOUNT,1000) = 0 THEN
COMMIT;
END IF;
EXIT WHEN drive_test_lat_long%NOTFOUND;
END LOOP;
CLOSE drive_test_lat_long;
COMMIT;
END drive_test_binning_single_zone;
|
|
|
|
| Re: Optimizing the procedure [message #427805 is a reply to message #426986] |
Sun, 25 October 2009 17:16 |
michael_bialik
Messages: 621
Registered: July 2006
|
Senior Member |
|
|
1. Still no bind variables.
2. Post TKPROF and explain plan.
3. Post some stats about the table.
4. You are selecting and updating the same table (may cause ORA-1555).
5. Try using BULK update.
HTH.
|
|
|
|
| Re: Optimizing the procedure [message #429799 is a reply to message #426986] |
Thu, 05 November 2009 17:42 |
 |
Kevin Meade
Messages: 2103
Registered: December 1999
Location: Connecticut USA
|
Senior Member |
|
|
this is basic stupidity.
I am not saying you are stupid so please do not take this as an insult. This is just a common mistake people make all the time.
query_str := 'UPDATE '
||p_binning_input_table
||' SET BIN_NAME='''
||binname
||''',BIN_LATITUDE='
||binlatitude
||',BIN_LONGITUDE='
||binlongitude
||' WHERE bin_name IS NULL AND latitude = '
||lrec.Latitude(i)
||' AND longitude='
||lrec.Longitude(i);
EXECUTE IMMEDIATE query_str;
END LOOP;
Execute Immediate is dynamic sql and all forms of dynamic sql always requires a parse each time they are done. There is no resuse in the cursor cache. Thus for every loop in your procedure you will be parsing an update statement, even if it is identical to the one that has come before. Bind variables mean nothing if you are doing this. You are parsing a statement for every row you fetch when what you should do is parse only once across the entire procedure call.
If you ever intend for this to go faster and for it to scale, then you must remove the use of execute immediate to do this update.
AS compared to this use of dynamic sql which is fine:
query_str VARCHAR(1000) := 'SELECT /*+ PARALLEL('
||p_binning_input_table
||', 32) */ longitude,latitude FROM '
||p_binning_input_table
||' GROUP BY longitude, latitude'; -- HAVING COUNT(*) > 1';
BEGIN
dbms_output.Put_line('INSIDE SINGLE ZONE');
binningfactor := p_bin_size;
OPEN drive_test_lat_long FOR query_str;
All dynamic sql requires a parse. But in this case you open the cursor only once so you incur no additional parsing overhead during the call. Once again bind variables mean nothing to this case.
That said, bind vairables are essential for getting code to scale so whatever new solution you devise it must include bind variables as needed.
Good luck, Kevin
[Updated on: Thu, 05 November 2009 17:45] Report message to a moderator |
|
|
|
Goto Forum:
Current Time: Fri Nov 29 05:05:22 CST 2024
|
] 
Blog
Search
Help
Members
Register
Login
Home
