I saw this self implemented bounded blocking queue
.
A change was made to it, aiming to eleminate competition by replacing notifyAll
with notify
.
But I don't quite get what's the point of the 2 extra variables added: waitOfferCount
and waitPollCount
.
Their initial values are both 0.
Diff after and before they're added is below:
Offer
:
My understanding is that the 2 variables purpose is that you won't do useless notify
calls when there's nothing wait
on the object. But what harm would it do if not done this way?
Another thought is that they may have something to do with the switch from notifyAll
to notify
, but again I think we can safely use notify
even without them?
Full code below:
class FairnessBoundedBlockingQueue implements Queue {
protected final int capacity;
protected Node head;
protected Node tail;
// guard: canPollCount, head
protected final Object pollLock = new Object();
protected int canPollCount;
protected int waitPollCount;
// guard: canOfferCount, tail
protected final Object offerLock = new Object();
protected int canOfferCount;
protected int waitOfferCount;
public FairnessBoundedBlockingQueue(int capacity) {
this.capacity = capacity;
this.canPollCount = 0;
this.canOfferCount = capacity;
this.waitPollCount = 0;
this.waitOfferCount = 0;
this.head = new Node(null);
this.tail = head;
}
public boolean offer(Object obj) throws InterruptedException {
synchronized (offerLock) {
while (canOfferCount <= 0) {
waitOfferCount++;
offerLock.wait();
waitOfferCount--;
}
Node node = new Node(obj);
tail.next = node;
tail = node;
canOfferCount--;
}
synchronized (pollLock) {
++canPollCount;
if (waitPollCount > 0) {
pollLock.notify();
}
}
return true;
}
public Object poll() throws InterruptedException {
Object result;
synchronized (pollLock) {
while (canPollCount <= 0) {
waitPollCount++;
pollLock.wait();
waitPollCount--;
}
result = head.next.value;
head.next.value = null;
head = head.next;
canPollCount--;
}
synchronized (offerLock) {
canOfferCount++;
if (waitOfferCount > 0) {
offerLock.notify();
}
}
return result;
}
}
You would need to ask the authors of that change what they thought they were achieving with that change.
My take is as follows:
Changing from notifyAll()
to notify()
is a good thing. If there are N
threads waiting on a queue's offerLock
or pollLock
, then this avoids N - 1
unnecessary wakeups.
It seems that the counters are being used avoid calling notify()
when there isn't a thread waiting. This looks to me like a doubtful optimization. AFAIK a notify
on a mutex when nothing is waiting is very cheap. So this may make a small difference ... but it is unlikely to be significant.
If you really want to know, write some benchmarks. Write 4 versions of this class with no optimization, the notify optimization, the counter optimization and both of them. Then compare the results ... for different levels of queue contention.
I'm not sure what "fairness" is supposed to mean here, but I can't see anything in this class to guarantee that threads that are waiting in offer
or poll
get treated fairly.