?
Solved

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

Posted on 2008-02-07
9
Medium Priority
?
171 Views
Last Modified: 2013-11-24
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

0
Comment
Question by:richardsimnett
9 Comments
 
LVL 10

Expert Comment

by:oleber
ID: 20848429
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
 
LVL 10

Accepted Solution

by:
oleber earned 2000 total points
ID: 20848456
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
 
LVL 13

Expert Comment

by:Bart Cremers
ID: 20848550
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
Never miss a deadline with monday.com

The revolutionary project management tool is here!   Plan visually with a single glance and make sure your projects get done.

 
LVL 10

Expert Comment

by:oleber
ID: 20848732
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
 
LVL 26

Expert Comment

by:Tomas Helgi Johannsson
ID: 20849405
   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
 
LVL 27

Expert Comment

by:mrcoffee365
ID: 20851426
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
 

Author Comment

by:richardsimnett
ID: 20851534
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
 
LVL 10

Expert Comment

by:oleber
ID: 20851585
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
 
LVL 10

Expert Comment

by:oleber
ID: 20851638
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

Featured Post

Receive 1:1 tech help

Solve your biggest tech problems alongside global tech experts with 1:1 help.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Java functions are among the best things for programmers to work with as Java sites can be very easy to read and prepare. Java especially simplifies many processes in the coding industry as it helps integrate many forms of technology and different d…
International Data Corporation (IDC) prognosticates that before the current the year gets over disbursing on IT framework products to be sent in cloud environs will be $37.1B.
This tutorial covers a practical example of lazy loading technique and early loading technique in a Singleton Design Pattern.
This theoretical tutorial explains exceptions, reasons for exceptions, different categories of exception and exception hierarchy.
Suggested Courses
Course of the Month6 days, 8 hours left to enroll

592 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question