Java CLI Application performance with StringBuilder

Question

General:
I am writing a socket client that receives "Market" data/quotes all the time (never ending loop) from some server side (distant one).
i am dividing the data in to chunks so i can use it.
each chunk contains about 200 characters and needs to be converted in to an array.
After a chunk was divided it is been parsed in to a List (No Problems here).

The problem:
The CPU usage is reaching to 40% after 10 minutes of running.
I have managed to isolate the problem.
Every chunk needs to be converted in to json.
So i am giving you now the actual code that does the problems.
this code executes every 300-400 MS.
skipping this code will leave the entire system with 1%-2% CPU usage.

Note:
I have read this thread but i don't see any solution there.
Is it better to reuse a StringBuilder in a loop?

The code:

private static StringBuffer jsonVal = new StringBuffer();
 public static String toJson(List<QuotesData> quotesData) {
 // Empty variable
 jsonVal.delete(0, jsonVal.length());
 jsonVal.append("{");
 synchronized (quotesData) {
 for (QuotesData quote : quotesData) {
 jsonVal.append("\"").append(quote.getSymbol()).append("\":[{");
 jsonVal.append("\"ask\":\"").append(quote.getAsk()).append(
 "\",");
 jsonVal.append("\"bid\":\"").append(quote.getBid()).append(
 "\",");
 jsonVal.append("\"time\":\"").append(quote.getDateTime())
 .append("\"}],");
 }
 jsonVal.append("}");
 String returnString = jsonVal.toString();
 return returnString.toString().replace("}],}", "}]}");
 }
 }

• leonbloy: Any reason you are using (contrarily to the title, BTW) a StringBuffer instead of StringBuilder ? Do you synchronization at that level ?

Answer 1

First I would suggest using JProfiler or JConsole, both included in JDK6, to pinpoint exactly where the performance hit is.

Without knowing where the CPU usage is, I would avoid synchronized. I doubt append is the problem. Clean it up by getting rid of the static local jsonVal, too.

public static String toJson(final List<QuotesData> quotesData) {
 final List<QuotesData> theData = new ArrayList<QuotesData>(quotesData);
 StringBuffer jsonVal = new StringBuffer();
 jsonVal.append("{");
 for (QuotesData quote : quotesData) {
 jsonVal.append("\"").append(quote.getSymbol()).append("\":[{");
 jsonVal.append("\"ask\":\"").append(quote.getAsk()).append(
 "\",");
 jsonVal.append("\"bid\":\"").append(quote.getBid()).append(
 "\",");
 jsonVal.append("\"time\":\"").append(quote.getDateTime())
 .append("\"}],");
 }
 jsonVal.append("}");
 String returnString = jsonVal.toString();
 return returnString.toString().replace("}],}", "}]}");
}

Consider using a JSON library like Gson. The code becomes much simpler. You can tweak the output if needed:

private static final Gson gson = new Gson();
public static String toJson(final List<QuotesData> quotesData) {
 return gson.toJson(new ArrayList<QuoteData>(quotesData));
}

• fatnjazzy: Thanks, List<QuotesData> is not clonable

Answer 2

My guest is that StringBuilder is constantly being resized. How many quotesData there is? I suggest you create a StringBuilder with a size before the for loop:

StringBuffer jsonVal = new StringBuffer(quotesData.size()*200); //the 200 is on top of my head. Do a few loop to see what is the average length of a QuotesData.

By the way, have you considered using StringBuilder instead? It's the same as StringBuffer, minus the overhead of being thread-safe (StringBuffer is synchronized, StringBuild is not).

Answer 3

OK, so this looks like a classic case of over optimization. Object creation isn't that expensive that you need to rewrite the same string buffer, especially if this is called every 300-400ms.

I'll try to address every possible scenario: Exponential growth The above code is assigned to a new thread every 300ms but the list is enormous and takes over 300ms to serialize. If this is the case you are basically choking your resources and it is only a matter of time before the application crashes. If this is the case, you should see the CPU constantly rising. The solution would be to:

1. Limit the number of threads that can run concurrently so you don't kill the application
2. Concurrently build the json object and merge the result so building a single json takes less then 300ms.

Speedups OK, so the list isn't clonable which I'm assuming means it isn't really a list but rather some sort of queue implemented as a list interface. So leaving synchronization as is I would do this:

public static final int JSON_LENGTH = 250; //you probably know this
public static String toJson(final List<QuotesData> quotesData) {
 jsonVal = new StringBuilder(JSON_LENGTH * quotesData.size());
 jsonVal.append("{");
 synchronized (quotesData) {
 for (QuotesData quote : quotesData) {
 jsonVal.append("\"").append(quote.getSymbol()).append("\":[{")
 .append("\"ask\":\"").append(quote.getAsk()).append("\",")
 .append("\"bid\":\"").append(quote.getBid()).append("\",")
 .append("\"time\":\"").append(quote.getDateTime()).append("\"}],");
 }
 // much much faster than replace
 jsonVal.setCharAt(jsonVal.length()-1, '}');
 return jsonVal.toString();
 }
}

Most of the changes are cosmetic, and I'm pretty sure that the JIT would already optimize them. Only difference I would do is use StringBuilder and create a new one each time and don't use .replace(). But to stress my point further unless you fit the first description (exponential growth) I doubt the problem is here. I would look at you list implementation first.

• fatnjazzy: Asaf, thanks... nothing seems to help. i even tried to do it an a thread pool... the funny thing is, i have the same code in PHP and it works faster for longer time. with no CPU problems... any way. Toda... Thanks

Answer 4

A few suggestions:

• Profile the code, it should show you the hot spots.
• Use StringBuilder instead of StringBuffer. StringBuffer is synchronized, StringBuilder is not.
• Is the synchronized statement really needed? If not, try removing it.
• toString() is not needed on the return statement. You can remove it.
• Modify the code so that you don't need the replace() method in the end, that could be costly if returnString is long.
• Try to create a new StringBuffer object before the loop instead clearing of the old one.
• Try to return interned value of the string i.e. return returnString.intern()

• fatnjazzy: yes, this is a thread safe area... buy i gave it a shot... no changes.