+2  A: 

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));
}
Jonathon
Thanks, List<QuotesData> is not clonable
fatnjazzy
@fatnjazzy thanks, you're right. I modified the answer.
Jonathon
A: 

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()
Juha Syrjälä
yes, this is a thread safe area... buy i gave it a shot... no changes.
fatnjazzy
A: 

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).

Thierry-Dimitri Roy
A: 

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.

Asaf
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
fatnjazzy