Should I cache System.getProperty("line.separator")?

Question

Consider such method:

@Override
public String toString()
{
    final StringBuilder sb = new StringBuilder();
    for (final Room room : map)
    {
        sb.append(room.toString());
        sb.append(System.getProperty("line.separator")); // THIS IS IMPORTANT
    }
    return sb.toString();
}

System.getProperty("line.separator") can be called many times.

Should I cache this value with public final static String lineSeperator = System.getProperty("line.separator") and later use only lineSeperator?

Or System.getProperty("line.separator") is as fast as using a static field?

Answer 1

I see your question as presenting a false dichotomy. I would neither call getProperty every time, nor declare a static field for it. I'd simply extract it to a local variable in toString.

@Override
public String toString()
{
    final StringBuilder sb = new StringBuilder();
    final String newline = System.getProperty("line.separator"); 
    for (final Room room : map) sb.append(room.toString()).append(newline);
    return sb.toString();
}

BTW I have benchmarked the call. The code:

public class GetProperty
{
  static char[] ary = new char[1];
  @GenerateMicroBenchmark public void everyTime() {
    for (int i = 0; i < 100_000; i++) ary[0] = System.getProperty("line.separator").charAt(0);
  }
  @GenerateMicroBenchmark public void cache() {
    final char c = System.getProperty("line.separator").charAt(0);
    for (int i = 0; i < 100_000; i++) ary[0] = (char)(c | ary[0]);
  }
}

The results:

Benchmark                     Mode Thr    Cnt  Sec         Mean   Mean error    Units
GetProperty.cache            thrpt   1      3    5       10.318        0.223 ops/msec
GetProperty.everyTime        thrpt   1      3    5        0.055        0.000 ops/msec

The cached approach is more than two orders of magnitude faster.

Do note that the overall impact of getProperty call against all that string building is very, very unlikely to be noticeable.

Answer 2

You do not need to fear that the line separator will change while your code is running, so I see no reason against caching it.

Caching a value is certainly faster than executing a call over and over, but the difference will probably be negligible.

Answer 3

If you have become aware of a performance problem that you know relates to this, yes.

If you haven't, then no, the lookup is unlikely to have enough overhead to matter.

This would fall under either or both of the general categories "micro-optimization" and "premature optimization." :-)

---

But if you're worried about efficiency, you probably have a much bigger opportunity in that your toString method is regenerating the string every time. If toString will be called a lot, rather than caching the line terminator, cache the generated string, and clear that whenever your map of rooms changes. E.g.:

@Override
public String toString()
{
    if (cachedString == null)
    {
        final StringBuilder sb = new StringBuilder();
        final String ls = System.getProperty("line.separator");
        for (final Room room : map)
        {
            sb.append(room.toString());
            sb.append(ls);
        }
        cachedString = sb.toString();
    }
    return cachedString;
}

...and when your map changes, do

cachedString = null;

That's a lot more bang for the buck (the buck being the overhead of an extra field). Granted it's per-instance rather than per-class, so (reference earlier comment about efficiency) only do it if you have a good reason to.