I need some suggestions on how to optimize this code for CPU usage....

Hello,
I have a servlet which I wrote for glassfish. It receives an XML request, and then queries data from the database and returns the result set in CSV format. Currently, the query (a stored proc) returns roughly 10000 rows of data in about 0.03 secs from the MYSQL server, the glassfish servlet sits at 100% cpu utilization for about 45 seconds (rendering the CSV I think), and then spits the 10000 rows of data to the data requester. This process takes entirely too long, and uses to much CPU cycles. This is just with 1 requester currently, and by spec I should be able to handle 100's of simultaneous requests. There is no way doing such a minimal task (its only 2 columns of data) being put into CSV format, should hit the CPU so hard or take so long, so I figure I must be doing something wrong. Please look at this code, and tell me how to improve its CPU performance overall with glassfish.

There are 2 classes of code to look at the servlet, and the glassfishDB class:

the servlet - this is the only place that the glassfishDB is referenced in the servlet.
--------------


....

                 glassfishDB db = new glassfishDB("data_pool");
                 String query = "call getData(" + campaignid + ",'" + number + "','" + request.getRemoteAddr() + "');";
                 db.executeQuery(query);
               
                 String output = db.getCSVRecords();
                 
                 //close the database
                 db.close();

                 //output the result set
                 out.println(output);

....

glassfishDB -- this is the part where I think optimization must be needed. (see the code snippet for the entire class)
---------------

synchronized String getCSVRecords()
        {
            String data = "";
                  try
            {

                  while (rs.next())
                  {
                           
                        for (int col = 1; col <= meta.getColumnCount(); col++)
                                   data += rs.getString(col) + ",";
                               
                                data = data.substring(0,data.length()-1);
                                data += "/n/n/n";
                  }
            }
            catch (Exception e)
            {
                  //System.out.println("Database Read Error: " + e.toString());
                        errorMessage = e.toString();
                        errorState = 4;
            }
           
           return data;
  }


Worth 500 points.

Thanks,
Rick

/*
 * Database.java
 *
 * Created on November 1, 2006, 1:28 AM
 *
 * To change this template, choose Tools | Template Manager
 * and open the template in the editor.
 */
 
 
 
//Database Connectivity - Requires ODBC
import java.util.Hashtable;
import javax.sql.*;
import java.sql.*;
import javax.naming.*;
 
public class glassfishDB {
	Statement stmt;
	ResultSet rs;
	ResultSetMetaData meta;
	Connection con;
	long numberofrows = -1;
        int errorState = 0;
        String errorMessage = "";
        String poolname = "";
        
	glassfishDB(String poolName)
	{
            this.poolname = poolName;
	}
 
        public static DataSource getDataSource(String dsName) throws NamingException 
        {
            InitialContext context = null;
            context = new InitialContext();
            DataSource dataSource = (DataSource) context.lookup("jdbc/" + dsName);
            return dataSource;
        }
        
	void executeQuery(String query)
	{
            try 
            {
                DataSource ds = getDataSource(poolname);
		con = ds.getConnection(); 
                stmt = con.createStatement();
                // Do db operations.
                // Do not close driver connection
		rs = stmt.executeQuery(query);
		rs.setFetchSize(1); 
		meta = rs.getMetaData();
	    }
	    catch (Exception e)
	    {
                //System.out.println("Database Connection Error:" + e.toString());
                errorMessage = e.toString();
                errorState = 1;
	    }
	}
 
	void executeUpdate(String query)
	{
            try 
            {
                DataSource ds = getDataSource(poolname);
                con = ds.getConnection();
                stmt = con.createStatement();
		stmt.executeUpdate(query);
            }
	    catch (Exception e)
	    {
		//System.out.println("Database Connection Error:" + e.toString());
                errorMessage = e.toString();
                errorState = 2;
            }
            finally
            {
                close();
            }
	}
 
	long getRowCount()
	{
		try
		{
			int start = rs.getRow();
			rs.last();
			int last = rs.getRow();
 
			rs.first();
 
			numberofrows = last - start;
		}
		catch(Exception e)
		{
			//System.out.println("Database getRowCount() Error: " + e.toString());
                        errorMessage = e.toString();
                        errorState = 3;
		}
          return numberofrows;
	}
 
        synchronized String getCSVRecords()
        {
            String data = "";
            	try
		{
 
			while (rs.next())
			{
                           
				for (int col = 1; col <= meta.getColumnCount(); col++) 
                                   data += rs.getString(col) + ",";
                                
                                data = data.substring(0,data.length()-1);
                                data += "/n/n/n";
			}
		}
		catch (Exception e)
		{
			//System.out.println("Database Read Error: " + e.toString());
                        errorMessage = e.toString();
                        errorState = 4;
		}
            
           return data;
        }
        
        synchronized Hashtable getNextRecord()
	{
		Hashtable ht = new Hashtable();
 
		//Assign the next record to the hashtable
		try
		{
 
			if (rs.next())
			{
				////System.out.println("Data Present");
				//loop over the variable names and assign them and their values to the hashtable
				for (int col = 1; col <= meta.getColumnCount(); col++) {
				   String varName = meta.getColumnName(col).trim();
				   ////System.out.println("ColNAME " +varName);
				   String tdata = rs.getString(col);
 
				   if (tdata == null)
				   	  tdata = "";
 
				   ht.put(varName, tdata);
				}
			}
			else
			{
				ht = null;
			}
		}
		catch (Exception e)
		{
			//System.out.println("Database Read Error: " + e.toString());
                        errorMessage = e.toString();
                        errorState = 4;
		}
 
		return ht;
	}
 
	void close()
	{
		try
                {
                    con.close();
                    if (rs != null)
                        rs.close();
                }
                catch (Exception e)
                {
                    System.out.print("Database Close Error: " + e.toString() + "\n\n");
                }
        }
 
}

Open in new window

richardsimnettAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

oleberCommented:
there are some fix that you can do but one seems clear:
'String data...' is bad really bad. Construct the String with the StringBuffer and you performance will increase allot.
0
oleberCommented:
String output = db.getCSVRecords();
....                
out.println(output);

the only use of the string is to be printed? consider to redo the 'getCSVRecords()' passing the 'out' as parameter if needed and doing a print line by line. You will avoid to create all the string in memory.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Bart CremersJava ArchitectCommented:
Constructing the String with a StringBuffer will probably decrease performance on Java 5 instead of increasing it. The compiler will probably optimize it anyway, but if you want to do the optimization yourself, at least use a StringBuilder.
(http://java.sun.com/developer/technicalArticles/Interviews/community/kabutz_qa.html)

Another thing is possibly the fetch size. Don't set it, let the JDBC driver and the database figure out the best size their selves. They're probably better at guessing it anyway.

For the rest it is very hard to find what is taking all CPU. You should profile the method for CPU and memory usage and find out what is taking all this time.

I can't see too much in the code, but my guess is that the process doesn't have enough memory available, so it needs to garbage-collect often. Tuning the heap and the garbage collector will increase performance significant.
0
Upgrade your Question Security!

Your question, your audience. Choose who sees your identity—and your question—with question security.

oleberCommented:
You are saying that using a StringBuffer instead of String will decrease the spead?
Are you joking, really are you joking?

a simple code like this:

            long time = System.currentTimeMillis();
            String match = "match";
            String all = "";
            for (int i=0; i<20000; i++) {
                  all += match;
            }
            System.out.println(System.currentTimeMillis() - time);

Compared to

            long time = System.currentTimeMillis();
            String match = "match";
            StringBuffer strbfr = new StringBuffer();
            for (int i=0; i<20000; i++) {
                  strbfr.append(match);
            }
            String all = strbfr;
            System.out.println(System.currentTimeMillis() - time);

runs 2000 times faster in my machine using java 1.6.

Read again what is sayed there, please and you will become better informed.

At most I can consider the StringBuilder as one option.
0
Tomas Helgi JohannssonCommented:
   Hi!

The line 116 seems to me to be an overkill when you create the CSV.
Comment out that line and try.

Regards,
   Tomas Helgi
0
mrcoffee365Commented:
In my testing with this program, oleber is completely correct.  Changing String data to StringBuffer data makes all the difference in the world.

There are some other things -- you probably want the line end to be "\n" not "/n/n/n" -- but you would have found that eventually.
0
richardsimnettAuthor Commented:
ok I have a working optimization... here is the solution I chose to use:

servlet:

glassfishDB db = new glassfishDB("data_pool");
String query = "call getData(" + campaignid + ",'" + number + "','" + request.getRemoteAddr() + "');";
db.executeQuery(query);

//output the result set
int count = db.getCSVRecords(out);
                 
//close the database
db.close();


glassfishDB
---------------

        synchronized int getCSVRecords(PrintWriter out)
        {
                int count = 0;
               
                  try
            {
                  while (rs.next())
                  {
                        for (int col = 1; col <= meta.getColumnCount(); col++)
                                    if (col == 1)
                                        out.print(rs.getString(col) + ',');
                                    else
                                        out.print(rs.getString(col) + "/n/n/n");
                               
                                ++count;
                  }
            }
            catch (Exception e)
            {
                  //System.out.println("Database Read Error: " + e.toString());
                        errorMessage = e.toString();
                        errorState = 4;
            }
           
                return count;
        }


Thanks for all the responses!
0
oleberCommented:
the optimization is just done in one command:
if you have

String str = str1 + str2;

the value of str is constructed with a StringBuffer/StringBuilder depending of the version of Java for what Bart_Cr is saying.

but when you do:

String str = str1 + str2;
str += str3;

Two StringBuffer/StringBuilder (one for each line) are created and both strings are being coyed.


The variable data is a big string, being copied 3 times for each row. So the CPU is just doing unnecessary copy of memory.
0
oleberCommented:
this seems to be wrong:

                        for (int col = 1; col <= meta.getColumnCount(); col++)
                                    if (col == 1)
                                        out.print(rs.getString(col) + ',');
                                    else
                                        out.print(rs.getString(col) + "/n/n/n");

try

                        for (int col = 1; col <= meta.getColumnCount(); col++)
                                    if (col < meta.getColumnCount())
                                        out.print(rs.getString(col) + ',');
                                    else
                                        out.print(rs.getString(col) + "/n/n/n");


Your code works if you get exactly 2 rows, and in this case you can do:

                        out.print(rs.getString(1)  + ',' + rs.getString(2) + "/n/n/n");

If I'm seeing this correctly ;)
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java EE

From novice to tech pro — start learning today.