0

We are using concatenated strings almost everywhere for logging purposes.
For eg: catch (Exception e) { logger.error("setUserSession()" + e.getMessage()); } But we doubt if these statements cause memory leaks because of string mutation. Some suggests to use String builder. But what I understood from discussions like StringBuilder vs String concatenation in toString() in Java is Java Compiler will convert string concatenations with the use of String builder.

To test this I have tried to decompile following java codes(using JD-GUI);

    logger.error("string1" + "string2");
    logger.error("setUserSession()" + e.getMessage());

And decompiler produced following code;

    logger.error("string1string2");
    logger.error("setUserSession()" + e.getMessage());

But I was expecting the String builder in second statement like ;

logger.error(new StringBuilder().append("setUserSession()").append(e.getMessage());

So I have 2 doubts here:

  1. Does string concatenation actually creates so much memory leaks?
  2. Or do I need to use String builder explicitly->append my strings to string builder->then delete string builder using string builder delete function?
  3. Java compiler converts simple string concatenations with String builder. (And we can use decompiler tools to check that?)
Community
  • 1
  • 1
Vivek Mohan
  • 8,078
  • 8
  • 31
  • 49

2 Answers2

5
  1. It won't create memory leaks. It will create memory usage, but the garbage collector will then tidy that up.
  2. There is no need to use StringBuilder explicitly unless your string concatenation is spread out over multiple statements.
  3. Yes.

I think you have a misunderstanding about Java though. It doesn't matter whether those objects are String, Object, MyObject or anything else. As soon as all strong references to it have gone out of scope the object is eligible for garbage collection and at an appropriate moment the memory used will be reclaimed.

The only way to leak memory in Java is either to leak resources (for example opening streams, creating threads, etc) or by accidentally keeping a reference to something (for example adding it to a List and never removing it from the List).

Tim B
  • 40,716
  • 16
  • 83
  • 128
  • Thanks Tim. There are discussions like String literals are stored in JVM perm gen space and garbage collection is very rare in that area. Also in my scenario of using logger, what will be better String concatenation or String builder? – Vivek Mohan Dec 30 '13 at 12:34
  • @Vivek since they do the same thing, I would use the one you believe is simpler. – Peter Lawrey Dec 30 '13 at 12:39
  • There is no point using the builder since it is just adding extra syntax to achieve the same result. – Tim B Dec 30 '13 at 12:53
  • So I can use my code like logger.error("setUserSession()" + e.getMessage()); assuming there will be no memory leak or performance problem... – Vivek Mohan Dec 30 '13 at 12:58
  • You can, although you are better off using a message template for this. See my answer here for why: http://stackoverflow.com/a/20840145/3049628 – Tim B Dec 30 '13 at 13:07
0

It is a good practice to check whether log level is enabled before log statement as follows:

if(logger.isErrorEnabled()) {

    //At runtime while executing below statement,Java creates "String1" and "String2"
    //String objects in the string pool regardless of the log level
    logger.error("string1" + "string2");
}

This prevents unnecessary creation of "String1" and "String2" objects in the String pool although logging statement does nothing.(In case log level is greate than ERROR)

Serdar
  • 383
  • 1
  • 9
  • You don't need to do that, just use the message template properly: http://stackoverflow.com/a/20840145/3049628 – Tim B Dec 30 '13 at 13:06