Performance issue

Hello EveryBody,

Hi Iam working on the application which usually get slow ,so performance issue is top priority for the application and we are advised to do a code review and I have find that some unused varaibles , Arrays ,Class Object and methods have been declared which are not use and i have felt that it should be removed from the code, moreover i have observed in the code that Object is getting typecast into String like this String name= (String) getName() now I have question we we had use getName().toString() will it make alot of difference or not .

Is tyepacsting takes more time then calling directly toString() or not.
Please help me where Iam wrong.

Thanks in advance

Removing unused variables is unlikely to do anything for performance, because the compiler and/or the JVM do that anyway.

Typecasting will generally be a lot faster than calling the toString() method.

Jasmine,
Changing those things helps for readability and maintenance, but not performance.

For performance, you are unlikely to find a bottleneck by just looking at the codebase. The first step should be finding what part of the code is taking the most time.

Such minor changes without locating a bottleneck will probably result in no performance improvements. This sounds like a case of premature optimization. See the javaranch link below for further information on unjustified microtuning.

From a performance standpoint I don't see much to do here, but I do have some comments on the code in general.

1) Production code should not have unused variables, and methods that have no real use. Whenever I see such sloppy code, other problems, such as bugs and poor design aren't far behind. Your team should come up with a checklist that developers should follow on their own code, even before bringing it to a code review. Removing unused variables should be one of them. This is basic professionalism.

2) I wouldn't decide on using a cast or toString() based on performance.

Both casting and calling toString() can have bad consequences under certain circumstances.

The following would throw an exception if myObj is not a String, however nulls will be ok
String str=(String) myObj;

the following will work with any Object but throw a null pointer exception if myObj is null.
String str=myObj.toString();

I suspect getName should return a String and not an Object. If that is the case then getName should be declared to return a String, and then you don't need to cast or call toString(). You could then simply call:

String name=getName();

If it is valid to have no last name then I would consider returning the empty string instead of a null for this case. This avoids callers of getName() from having to check and see if a null is returned to avoid a null pointer exception.

3) In general get the code right, then monitor it for performance and tune if needed.
[ February 20, 2007: Message edited by: steve souza ]

[Jeanne]: Changing those things helps for readability and maintenance, but not performance.

Definitely agreed. (And with Steve.) Except that readability can help performance, and correctness, because it makes it easier to see if you are doing something wrong, or doing something inefficiently. This is an indirect effect, but it can be substantial. So Jasmine, don't underestimate the importance of readability here. However it is extremely unlikely that any of the things you mention in your post are the primary cause of your current performance problems. Note that Steve's signature contains links to two excellent resources for further information about performance tuning.

Hello EveryBody ,
Thanks for helping me and giving the right direction ,well iam following the things which has been shared by Steve and I have more questions which i would like to ask which I have observed in the code.
1) I have observe that unused array objects and Class object have been declared in the code so it should be removed from the code .

2) I have observed that while exception handling they have created the object of GenericCmdAppError in every method in the class , so my question is this cant we create an object of GenericCmdAppError once in the class use the refernce of that object .Tell me whether Iam right or not.

3) In the code we are closing connection Object in some of the methods whereas ResulSet and PreparedStatement objects are getting closed everywhere.So Connection object should be closed in every transaction , tell me Iam right or not.

Thanks once again and I would like to have amore discussion so that I can have a better undesrtanding in the performance issue which I have analysed so far and can improve the code.
Thanks

1) I have observe that unused array objects and Class object have been declared in the code so it should be removed from the code.

For clarity you can remove them, but it's not going to have an impact on performance.

2) I have observed that while exception handling they have created the object of GenericCmdAppError in every method in the class, so my question is this cant we create an object of GenericCmdAppError once in the class use the refernce of that object.

Possibly, but it depends on what GenericCmdAppError (about which we know nothing) does. Also, object creation does not imply the big overhead that it used to in earlier versions of the JDK. Assuming that error conditions are rare, this might not make a difference in performance.

3) In the code we are closing connection Object in some of the methods whereas ResulSet and PreparedStatement objects are getting closed everywhere.So Connection object should be closed in every transaction , tell me Iam right or not.

Closing Connection also closes Statement and ResultSet. But there is no need to close a connection just because the transaction is completed. In fact, opening a connection carries a definite overhead that is generally avoided by using connection pools (which keep connections open all the time).
[ February 21, 2007: Message edited by: Ulf Dittmer ]

Well I would like to discuss when we create an arrays and class object in the code it takes memory and same thing goes with unused global variables , as they get memory when object creation takes place so we should remove it.

I have also observed that System.out.println(""); statement is also been used and should be remove as it unnecessary makes an overhead tell me if Iam wrong

The another thing while analysing the code is that somewhere the connection object is getting created by the given below method on the Class whereas some where we are creating a connection Object by using
thedbhandle = getCommandContext().getDBHandle();
con = dbhandle.makeConnection();
------------------------------------------------------------------------

public Connection getConnectionObject()
{
try
{
dbhandle = getCommandContext().getDBHandle();
con = dbhandle.makeConnection();
if(!con.equals(null))
ExceptionSource1 = true;
}
catch(Exception _ex) { }
return con;
}
So please tell is declaring amethod getConnectionObject()in a class to get aconnection Object is OK or not or should the developer should create the connection object by using dbhandle = getCommandContext().getDBHandle(); and handling exception their itself.

Correct me if Iam wrong anywhere and I need to clear my concepts.

Thanks alot for advising me.

Well I would like to discuss when we create an arrays and class object in the code it takes memory and same thing goes with unused global variables , as they get memory when object creation takes place so we should remove it.

Yes, you should have these probems fixed.

I have also observed that System.out.println(""); statement is also been used and should be remove as it unnecessary makes an overhead tell me if Iam wrong

I think you are focusing on the wrong thing. Most of the issues you mention are not performance problems, HOWEVER they are bad coding practices. The println doesn't make sense and should probably be removed. However, the bigger problem is not abstracting logging. For now you could create your own application logger class (Logger.log("")) and later replace System.out.println's in your program with a more powerful logging package like log4j.

As far as your comments on connections I am not sure what you are asking. However, you don't want every consumer class coding connection logic. Create a dbutility class to handle that and abstract this as much as possible.

There are a number of problems with the code.

1) if this can every happen it will always throw a null pointer exception.
if(!con.equals(null))

2) It is a terrible practice to catch an Exception, and do nothing with it (especially one as bad as not getting a database connection). At a minimum the exception should be logged.
catch(Exception _ex) { }

As others have said, concentrating on the small coding issues you talk about is unlikely to gain much in terms of performance, though fixing them would make the code a lot more reliable, scalable, maintainable etc. To get big improvements, you need to look at the bigger picture and see if the algorithms and architecture choices are efficient. You also need to use measurement and profiling to identify what's really taking the time.

To put a (sort of) positive spin on it: if much of the code base you're working on has code as bad as the extract that you've posted, then there are likely to be plenty of opportunities for performance improvement! People who think you should just catch and squash Exception probably can't design a decent algorithm or architecture.

Iam following the advise which you advise me.
Well Iam sharing you the architecture of the application which are following.
1) This application is designed on Command based and on struts framework ,I mean it has a mixed architecture since its a web based application , JSP, ,HTML, VO(Value Object), BO(Business Object) , java script has been used and oracle database .
this is the architecture which have used to design the apllication and application is deploy on WAS.No design pattern have been used in the application
2)Well tell me will the mix architecture of Command base and struts will make any difference or not ?
3)I have one question related to this architecture what is the use of Command base architecture over struts .
If we use simple architecture to design an application when we should use and how it is better then complex architecture as when we think to design an application what parameters we should keep in mind to select an architecture for any small , mid size or big application as I know I have seen where major transactions takes place which are compex usually EJBS have been used .What is the limitation of simple architecture
Thanks alot

As already said, firstly you have to find out where is the problem, and then what is the problem, and then try to figure out the appropriate solution. A rational analysis is needed.

Some questions:
- Check whether it is because of damaging unfavorable SQL Queries?
- What are you presently doing to cater this issue? Restarting server or something?
- Is this occurs only in particular areas of application? like reports and heavy queries.