OraFAQ Forum: Performance Tuning » Optimizing the procedure

Home » RDBMS Server » Performance Tuning » Optimizing the procedure (10.2.0.4, Solaris 10)

Show: Today's Messages :: Polls :: Message Navigator
E-mail to friend

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;

Report message to a moderator

Re: Optimizing the procedure [message #426989 is a reply to message #426986]

Tue, 20 October 2009 08:48

cookiemonster
Messages: 13923
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.

Report message to a moderator

Re: Optimizing the procedure [message #426994 is a reply to message #426986]

Tue, 20 October 2009 09:01

cookiemonster
Messages: 13923
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.

Report message to a moderator

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.

Report message to a moderator

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.

Report message to a moderator

Re: Optimizing the procedure [message #426998 is a reply to message #426995]

Tue, 20 October 2009 09:16

khan500
Messages: 2
Registered: October 2009

Junior Member

It uses a single table, also implementing forall is ruled out since just above the update statment I do some processe for the records in the loop.

Report message to a moderator

Re: Optimizing the procedure [message #426999 is a reply to message #426986]

Tue, 20 October 2009 09:31

cookiemonster
Messages: 13923
Registered: September 2008
Location: Rainy Manchester

Senior Member

If you're doing it for one table why is the table name dynamic?

Report message to a moderator

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.

Report message to a moderator

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 #427791 is a reply to message #427782]

Sun, 25 October 2009 09:43

Michel Cadot
Messages: 68653
Registered: March 2007
Location: Nanterre, France, http://...

Senior Member
Account Moderator

Keep your lines in 80 characters!

Regards
Michel

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;

Report message to a moderator

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.

Report message to a moderator

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

Previous Topic:	How to Query Large table .?
Next Topic:	optimisation

Goto Forum:

-=] Back to Top [=-

[ Syndicate this forum (XML) ] [

]

Current Time: Sat May 18 07:06:22 CDT 2024