user2916610
user2916610

Reputation: 765

Can I use Collection.size() to replace the counter in this code?

Here's the code:

public class LogService {
    private final BlockingQueue<String> queue;
    private final LoggerThread loggerThread;
    private final PrintWriter writer;
    @GuardedBy("this") private boolean isShutdown;
    @GuardedBy("this") private int reservations;    //  <-- counter
    public void start() { loggerThread.start(); }
    public void stop() {
        synchronized (this) { isShutdown = true; }
        loggerThread.interrupt();
    }
    public void log(String msg) throws InterruptedException {
        synchronized (this) {
            if (isShutdown)
                throw new IllegalStateException(...);
            ++reservations;
        }
        queue.put(msg);
    }
    private class LoggerThread extends Thread {
        public void run() {
            try {
                while (true) {
                    try {
                        synchronized (LogService.this) {
                            if (isShutdown && reservations == 0)
                                break;
                        }
                        String msg = queue.take();
                        synchronized (LogService.this) {
                            --reservations;
                        }
                        writer.println(msg);
                    } catch (InterruptedException e) { /* retry */ }
                }
            } finally {
                writer.close();
            }
        }
    }
}

It's a snippet from the book Java Concurrency in Practice, and I'm thinking about that maybe the counter reservations is unnecessary as we could simply use queue.size() to get the number of elements in queue.

Am I right?

Upvotes: 8

Views: 91

Answers (2)

Ashkrit Sharma
Ashkrit Sharma

Reputation: 637

Using "reservations" variable is good domain design because it is much more meaningful than size, you can use that to express domain concept like total number of reservation available etc

On the performance side

ArrayBlockingQueue - calling size function will result in slowness because it acquires lock to read the size and that will slow down take or put operation also.

LinkedBlockingQueue - Calling size is atomic/volatile read and it has performance cost.

Upvotes: 0

djechlin
djechlin

Reputation: 60788

No, that would create a deadlock actually.

You would need to synchronize put and take if you want to use size in a parallel fashion. But take is blocking and you would now have a blocking take call synchronized on the same object as the put call. take can't take until something is put. put can't put until take gives up the lock. That's a deadlock.

Upvotes: 5

Related Questions