How bad is new Thread().sleep compared to Thread.sleep in terms of CPU and memory utilization?

Question

I'm aware that sleep should be accessed in a static context. However I need more inputs so I can defend this to the management. Most of the legacy code that I'm handling now uses new Thread().sleep instead of Thread.sleep.

How bad is this?

for (int c = 0; c < 5; c++) {
    new Thread().sleep(5000);
}

compared to this?

for (int c = 0; c < 5; c++) {
    Thread.sleep(5000);
}

EDIT:

    final long start = System.currentTimeMillis();
    System.out.println("Total memory: " + Runtime.getRuntime().totalMemory());
    System.out.println("Free memory: " + Runtime.getRuntime().freeMemory());
    System.out.println("===========================");
    for (int c = 0; c < 5; c++) {
        new Thread().sleep(5000);
        System.out.println("Used memory: " + (Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory()));
        System.out.println("===========================");
    }
    System.out.println("Total memory: " + Runtime.getRuntime().totalMemory());
    System.out.println("Free memory: " + Runtime.getRuntime().freeMemory());
    System.out.println("===========================");
    System.out.println("Time elapsed: " + (System.currentTimeMillis() - start) + " milliseconds");

Result (new Thread().sleep):

Total memory: 5177344
Free memory: 4990904
===========================
Used memory: 186440
===========================
Used memory: 205136
===========================
Used memory: 205136
===========================
Used memory: 205136
===========================
Used memory: 205136
===========================
Total memory: 5177344
Free memory: 4972208
===========================
Time elapsed: 24938 milliseconds

Result (Thread.sleep):

Total memory: 5177344
Free memory: 4990904
===========================
Used memory: 186440
===========================
Used memory: 186440
===========================
Used memory: 204848
===========================
Used memory: 204848
===========================
Used memory: 204848
===========================
Total memory: 5177344
Free memory: 4972496
===========================
Time elapsed: 24938 milliseconds

Answer 1

Actually it costs quite a bit:

new Thread() is, in fact, an expensive call (not as much as Thread.start(), of course) but involves stack crawl, few synchronized blocks, few security checks, copy of the inherited thread locals and so. Under security manager it can get messy as well.

Prior to 1.3 the unstarted threads used to cause a severe memory leak... Now it's still an issue: https://bugs.java.com/bugdatabase/view_bug?bug_id=4229558 ThreadGroup reports wrong threadCount AND can become negative... but worst of all still causes leak since the ThreaGroup won't puff (destroy()) after all threads get exit unless discarded by hand. The latter can be problematic as it requires to wait for all the threads to exit properly and after that call destroy(). Sounds simple but it's an actual problem for managed environments (namely the container).

Edit Since it's not clear ThreadGroup can be subclasses in order to override uncaughtException(Thread t, Throwable e) and deal with the throwable. That makes the ThreadGroup class holding reference to the classloader and the issue is real. (not just a minor single object leak)

Overall unstarted threads have been bugging Java since its inception, you'd be better off replacing the code. It will take less time than I wrote the reply (mostly to shed some light to the believed free initialization).

Answer 2

In addition to what has already been said: if you are already rewriting some legacy code, probably this is the most idiomatic and straightforward way to express sleep (you can of course use different time units):

TimeUnit.SECONDS.sleep(5);

Answer 3

How bad is it? Probably not really bad from a performance perspective.

My understanding is that the expensive part of thread instantiation only happens when you call Thread.start(). However, quite a bit of work happens during construction of the Thread object:

• initializing a bunch of fields,
• doing security checks,
• etcetera.

If you want evidence that it is a significant performance issue, then you'll need to profile your application. (And I have my doubts that it would be significant.)

But it shouldn't be hard / expensive thing to fix ... unless you are in a world where every change is hard / expensive ... or unless your application does some seriously weird / broken things with ThreadGroups or inheritable ThreadLocals.

My main complaint would be that it is as ugly as the north end of a south-bound cow. On a par with calling new String(...) all over the place.

---

I don't think your benchmark shows much. The overhead of creating an unwanted Thread is likely to be in the area of a micro-second or so. That's down in the noise. Besides, your methodology for measuring memory usage is plain wrong. The totalMemory() and freeMemory() methods give you memory stats that were recorded at the last GC run.

Answer 4

They do the same thing, and the method is static in both cases.

In new Thread().sleep(5000);, it's not clear what's actually going on, because the static method is being called from an instance. That is, it's functionally identical to Thread.sleep(5000);, which has the benefit of being transparent.

It's also not clear that it's the current thread that will be put to sleep, so someone could legitimately puzzle whether it's the current thread or a newly created thread which will be put to sleep; it's actually the current thread.

If the first style is allowed, you could come have something like:

Thread anotherThread = new Thread();

then later on have

anotherThread.sleep(5000);

This too is identical to Thread.sleep(5000);, and actually puts the current thread to sleep. But it's not at all obvious what's going on.

The style in the second case is preferable because it avoids these pitfalls, and any performance penalty associated with creating a new instance. It should therefore be used at all times.

Answer 5

If I understand how Java handles static methods correctly, the first method will still call the static version of sleep() in addition to creating a new thread object. It's almost always better to use the second because it's understood that the method is static by calling it with respect to the class rather than to an instance.

if you're able to quickly find/replace these code statements, I'd say it's worth it. However, since both codes work I don't think this is something you should be overly worried about.

Answer 6

It is just illiterate programming, simple as that. Same applies to calling any static method that way, it's not confined to Thread.sleep(). It imposes pointless space and time costs, but worse it betrays a major conceptual misunderstanding on the part of the programmer. I wouldn't devote much energy to going back and fixing all the occurrences but I would certainly do something about re-educating the personnel concerned.