Is it efficient to obtain a Logger from a static final variable initializer?

Question

We have a lot of class code which has some boilerplate like the following:

private static Logger logger = null;

private static Logger getLogger() {
  if (logger == null) {
    logger = Logger.getLogger(MyClass.class);
  }
  return logger;
}

The idea is that the class can log debug stuff into the Logger. The first code which needs to log something invokes getLogger() and that brings the logger into existence.

There are a couple of things I don't like about this pattern. First the singleton getLogger() is not synchronized and synchronizing it, while correct would put a burden on each subsequent call for no reason.

I really want to be able to condense it down to just this:

private static final Logger logger = Logger.getLogger(MyClass.class);

Then I can just reference logger directly and not even bother with a singleton getter.

The problem I fear is that by doing this I cause a Logger to be created when the class is loaded even if the logger is never called. I have 10,000+ odd classes all calling getLogger(), so how many instances of Logger am I actually creating here? If my log4j properties contains a few appenders am I just referencing the same logger over and over, or I am I creating 10,000 of these things?

I am considering thread safety. That's why I want to get rid of the broken singleton and rely on the inherently thread safe class loader. — locka, Sep 06 '11 at 11:56
AFAICS the problem is, that if two threads try to fetch the logger in the same time, two `Logger` instance will be created. — KARASZI István, Sep 06 '11 at 12:26
Yes that's the issue in the original boiler plate. However by moving it to an initializer the problem wouldn't occur because the classloader is synchronized so that if two threads access the class simultaneously, the statics will be created on one thread before the other gets to reference it. — locka, Sep 06 '11 at 12:33

score 2 · Answer 1 · answered Sep 06 '11 at 11:13

2

It will only create the objects if the class is actually initialized, i.e. if it's used. At that point, does the small overhead of a single object per class really matter that much?

Simplest answer: try it both ways, and see if you can observe any significant differences in performance / memory / etc. I doubt that you'll be able to, but if you can, you'll be able to decide then based on data what the most appropriate course of action is.

answered Sep 06 '11 at 11:13

Jon Skeet

1,421,763
867
9,128
9,194

For testing sake, I would suggest using the logger variable in each class, that will make performance/memory difference more visible when comparing. – mohdajami Sep 06 '11 at 11:18

score 2 · Accepted Answer · edited May 23 '17 at 12:04

If you use the default Log4j configuration (i.e. default LoggerRepository, DefaultCategoryFactory etc.) then you will create 10'000 Logger instances. How much memory do they consume? No one except God and your Profiler knows this. (And my guess only the latter one would tell that to you).

If the memory footprint would be too much for your environment, move Logger initialization to the static inner class like this:

static class LoggerHolder {
  static Logger logger = Logger.getLogger(MyClass.class);
}

private static Logger getLogger() {
  return LoggerHolder.logger;
}

That way the instance of Logger will be only created on the first getLogger call. (This technique is known as the Initialization On Demand Holder (IODH), it is thread-safe and has zero synchronization overhead).

And may I give you one offtopic suggestion? Consider to replace Log4J with the combination of SLF4J+Logback libraries. They are written by the very same authors and described as "a successor to the popular log4j project, picking up where log4j leaves off". You can read more in this SO thread.

Great answer +1, one point though if I have 10,000 extra classes to hold my logger instances then don't I run the risk of putting pressure on the PermGen space? — locka, Sep 06 '11 at 12:24
@locka Yes, 10000 additional classes will occupy some space in PermGen space. But again only memory Profiler will tell you the real numbers… I created a sample project with just 10'000 dummy classes with IODHs for Logger. And call getLogger() for all of them one by one. Mine OSX JVM consumed ~50Mb for PermGen, not a small amount, but also not a crucial one. (And if you don't call getLogger, inner class will not be loaded). — Volo, Sep 06 '11 at 15:59

score 1 · Answer 3 · answered Sep 06 '11 at 11:14

You are creating a separate logger instance for each class you load, however I believe they have a small memory footprint. Log4J is known to be optimized extensively, and I doubt they haven't thought about memory usage.

Overall, if you have 10K different classes, your app is huge - I doubt you will notice any significant difference in memory consumption. But the best is of course to measure it both ways, in your concrete environment.

score 1 · Answer 4 · answered Sep 06 '11 at 20:12

OP's unsynchronized getLogger() method is ok, ince Logger is thread safe. (hopefully. Since Logger is mutable, and designed to be used concurrently, I'll be surprised if its reference cannot be published safely without additional synchronization.)

Occasionally creating 2 loggers for the same class isn't a problem either.

The concern of memory usage isn't necessary though. Having an extra object for each class shouldn't be a concerning overhead (hopefully the Logger object is not too huge)

fmucar · Answer 5 · 2011-09-06T11:31:13.100

0

Static variables initialized once and same object is used for all instances. You don't really need that private static Logger getLogger() method for singleton pattern.

Just will make it lazy loaded but don't think that it is a big gain. Thounsands of objects are created and destroyed in really small amounts of time.

edited Sep 06 '11 at 11:31

answered Sep 06 '11 at 11:17

fmucar

14,361
2
45
50

score 0 · Answer 6 · answered Sep 06 '11 at 11:17

The way you want it (static final field) is what is usually done, and recommended by PMD. If you have a logger, there is a good chance it's used, so initializing it lazily won't gain anything in terms of memory, and certainly not much in performance.

I would capitalize it LOGGER, though, since it's a constant.

Is it efficient to obtain a Logger from a static final variable initializer?

6 Answers6