Check your loops

Codewithin a FOR, WHILE, or simple loop executes more than once (usually), so any inefficiency in a loop's scope therefore tends to have a multiplying effect. In one tuning exercise for a client, I discovered a 30-line function that ran in less than half a second, but was executed so frequently that its total elapsed time for a run was five hours. Focused tuning on that one program reduced its total execution time to less than twenty minutes. Always go to your loops first and make sure you are not introducing such a problem.

Here is an obvious example. My procedure accepts a single name argument, then processes each record fetched from a packaged cursor:

PROCEDURE process_data (nm_in IN VARCHAR2) ISBEGIN FOR rec IN my_package.my_cursor LOOP process_record ( UPPER (nm_in), rec.total_production); END LOOP;END;

The problem with this code is that I apply the UPPER function to the nm_in argument for every iteration of the loop. That is unnecessary because the value of nm_in never changes. I can easily fix this by declaring a local variable to store the uppercased version of the name:

PROCEDURE process_data (nm_in IN VARCHAR2) IS v_nm some_table.some_column%TYPE := UPPER (nm_in);BEGIN FOR rec IN my_package.my_cursor LOOP process_record (v_nm, rec.total_production); END LOOP;END;

Of course, it is not always so easy to spot redundant code execution. In this example, one would assume that I uppercased the name either because I knew for certain that process_record would not work properly with a lower- or mixed-case string, or because I was not really sure how process_record works and therefore "took out insurance" to head off any possible problems.

If I have found the process_data procedure to be a bottleneck, it is very important that I understand how all of the code on which it depends works. An incorrect assumption can intersect in very nasty ways with the algorithms of underlying programs. It may well be the case, for example, that process_record always performs an uppercase conversion on its first parameter. That would make my UPPER unnecessary.

Here is another program with lots of processing inside the loop. Do you see any unnecessary execution?

1 CREATE OR REPLACE PROCEDURE process_data (tab_in IN VARCHAR2) 2 IS 3 cursor_id PLS_INTEGER; 4 exec_stat PLS_INTEGER; 5 BEGIN 6 FOR rec IN (SELECT ... FROM ...) 7 LOOP 8 cursor_id := DBMS_SQL.open_cursor; 9 10 DBMS_SQL.parse (cursor_id,11 'SELECT ...12 FROM employee E, ' || tab_in ||13 'WHERE D.department_id = ' || rec.id ||14 'AND ...',15 DBMS_SQL.native16 );17 exec_stat := DBMS_SQL.execute (cursor_id);18 DBMS_SQL.close_cursor (cursor_id);19 END LOOP;20 END;

The problem is more subtle in this case. You need to understand the workings of DBMS_SQL (the dynamic SQL built-in package described in Chapter 15) to realize that:

· You can reusecursors allocated by the DBMS_SQL.OPEN_CURSOR function. Consequently, you do not need to open and close a cursor with each iteration of the loop.

· More importantly, whenever you're working with dynamic SQL, you must closely analyze the dynamic SQL string. In this case, when I look more closely at lines 11 through 14, I realize that the only thing changing with each loop iteration is the value of the department ID. Currently, this is being concatenated into the string, which means that this query will be reparsed for every different value of the ID. A much more efficient approach is to use binding, as I show in the following example.

Here is a process_data replacement that executes only what is necessary within the loop:

CREATE OR REPLACE PROCEDURE process_data (tab_in IN VARCHAR2)IS cursor_id PLS_INTEGER; exec_stat PLS_INTEGER;BEGIN cursor_id := DBMS_SQL.open_cursor; DBMS_SQL.parse ( cursor_id, 'SELECT ... FROM employee E, ' || tab_in || 'WHERE D.department_id = :my_ID AND ...', DBMS_SQL.native ); FOR rec IN (SELECT ... FROM ...) LOOP DBMS_SQL.bind_variable (cursor_id, 'my_id', rec.id); exec_stat := DBMS_SQL.EXECUTE (cursor_id); END LOOP; DBMS_SQL.close_cursor (cursor_id);END;