1
\$\begingroup\$

This is a follow-up question to this question.

Is this a reasonable implementation for a blocking version of method ArrayBlockingQueue.drainTo(Collection)?

/**
 * This blocking method blends {@link #take()} with {@link #drainTo(Collection)}.
 * <br>
 * Chiefly, {@code take()} blocks and {@code drainTo(Collection)} does not block.
 */
public int drainAtLeastOneTo(Collection<? super E> c)
throws InterruptedException {
    final ReentrantLock lock = this.lock;
    lock.lockInterruptibly();
    try {
        while (count == 0) {
            notEmpty.await();
        }
        final int x = drainTo(c);
        return x;
    }
    finally {
        lock.unlock();
    }
}

To clarify: This code comes from the Sun JDK source code of ArrayBlockingQueue. I simply blended the source from take() with drainTo(Collection).

I am considering adding it to a modified version of ArrayBlockingQueue.

\$\endgroup\$
  • \$\begingroup\$ What is notEmpty? How much of your code is a rewrite of ArrayBlockingQueue? Your lock lock is completely useless too, each time a thread calls the method they will get a new lock instance, so you have not implemented any locking at all..... your code does not work... though I suspect you did not know that when you posted it. \$\endgroup\$ – rolfl Aug 28 '15 at 4:17
  • \$\begingroup\$ I realize that your lock is a shadow-name of the instance lock. That's a bad practice - using the same names in methods variables as instance variables... my first comment above is "wrong". \$\endgroup\$ – rolfl Aug 28 '15 at 4:37
1
\$\begingroup\$

Your problem is easily solved without having to delve in to the internals of ArrayBlockingQueue, and without needing a new ReEntrantLock either. SImply use the methods that are currently available, and combine them....

public int drainAtLeastOneTo(Collection<? super E> c) 
        throws InterruptedException {
    c.add(take());
    return 1 + drainTo(c);
}
\$\endgroup\$
  • \$\begingroup\$ This is an interesting idea. I will consider it. However, your solution requires the caller to obtain (and free) the lock twice. Mine only requires it once. Why do I care? I have an situation where the producer is relatively much faster than the consumer. Thus, the consumer is sometimes starved on take(). My idea was to "take more" with this method. \$\endgroup\$ – kevinarpe Aug 28 '15 at 6:34
  • \$\begingroup\$ @kevinarpe then maybe a different queue is the answer \$\endgroup\$ – ratchet freak Aug 28 '15 at 8:20
  • \$\begingroup\$ @kevinarpe - about the double-kick at the lock can, yes, I lock it twice, but, for the record, you do too... just you nest yours instead. drainTo has an internal lock, and nesting lock blocks may not be free either. Will need a performance test to check if it has a significant impact. \$\endgroup\$ – rolfl Aug 28 '15 at 15:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.