Writing after long gap, looks like i lost the motivation or ran out of topic :-)
Recently while going through code of my current assignment, i noticed that java blocking/concurrent queue is used in worst possible way and that motivated me list down some of the bad practices to access queues.
- Ignoring return value of "offer" function.
Blocking queues has various function that can be used to insert(add/offer/put) and offer is the special one
Details from java doc about offer
Inserts the specified element into this queue if it is possible to do so immediately without violating capacity restrictions, returning true upon success and false if no space is currently available. When using a capacity-restricted queue, this method is generally preferable to add(E), which can fail to insert an element only by throwing an exception.
Return value is very important for offer function. In many case returned value is ignored and it becomes mystery that why some message are not added and this makes application magical that some time it works.
Offer function is really useful function & java Executors framework is using for very good reason. It is used for rejecting task when task submission rate is higher than task processing, below is the code snippet from ThreadPoolExecutor.java
So when you are using offer function never ignore return value. If you just want to throw some exception when ever queue is full then use "add" function, it has built in support to throws exception for capacity violation.
- Using IsEmpty/peek in spin loop
Blocking queue has lock based waiting strategy , it is used for blocking operation put/take , but it also has some function that are not blocking and if those are used in loop then it will cause heavy contention.
I found example where consumer was highly optimized , it was using isEmpty in loop to check whether some element is available or not and incase when there is an element then it will call take to retrieve element.
I don't know what developer was thinking , may be he was in very deep thought when such type of code was written.
Such consumer code caused dead lock, producer could not add element because it is unable to get lock and most of the time lock was acquired by isEmpty function.
For such type of code thread dump is also interesting, you will see producer blocked but no trace of consumer thread. Multiple thread dump are required to confirm this.
Blocking queue should be used in why it is designed to , in case if you use non blocking calls then you must have some backoff strategy to avoid dead lock.
- Single consumer calling take in loop
This is very common pattern, message are taken from queue one by one, since blocking queue is lock based it needs lock for each operation.
It is always better to get multiple items from queue in one call by using drainTo type of function, this will reduce contention will give significant performance gain.
- size function for heart beat
This is also very common case where size is use to print heartbeat message. Size is not cheap operation for blocking queue, it needs lock to get size.
Atomic variable can be used to keep track of queue size and read to atomic variable will not have any effect on queue performance.
- Using sleep waiting strategy with ConcurrentLinkedQueue
Code using sleep waiting strategy
I am sure you will have very good reason to use CLQ, but sleep spoils every thing because CLQ does't have blocking support , so many application will use sleep to avoid spin loop.
Sleep based approach fine in some scenario, but if you really want to build event based system then sleep does't help,you need some kind of notification mechanism, so when ever element is added to queue it will notify consumer about that.
It is pretty easy to add such support to CLQ. I created blocking concurrent queue to try few things, executor-with-concurrentlinkedqueue blog has some details about it.
Conclusion
Each type of queue is designed for very specific use case and it should be used in proper way.
Some of the bad practice mention in blog makes application slow/unresponsive, bad usage pattern should be avoided.
Recently while going through code of my current assignment, i noticed that java blocking/concurrent queue is used in worst possible way and that motivated me list down some of the bad practices to access queues.
- Ignoring return value of "offer" function.
Blocking queues has various function that can be used to insert(add/offer/put) and offer is the special one
Details from java doc about offer
Inserts the specified element into this queue if it is possible to do so immediately without violating capacity restrictions, returning true upon success and false if no space is currently available. When using a capacity-restricted queue, this method is generally preferable to add(E), which can fail to insert an element only by throwing an exception.
Return value is very important for offer function. In many case returned value is ignored and it becomes mystery that why some message are not added and this makes application magical that some time it works.
Offer function is really useful function & java Executors framework is using for very good reason. It is used for rejecting task when task submission rate is higher than task processing, below is the code snippet from ThreadPoolExecutor.java
So when you are using offer function never ignore return value. If you just want to throw some exception when ever queue is full then use "add" function, it has built in support to throws exception for capacity violation.
- Using IsEmpty/peek in spin loop
Blocking queue has lock based waiting strategy , it is used for blocking operation put/take , but it also has some function that are not blocking and if those are used in loop then it will cause heavy contention.
I found example where consumer was highly optimized , it was using isEmpty in loop to check whether some element is available or not and incase when there is an element then it will call take to retrieve element.
I don't know what developer was thinking , may be he was in very deep thought when such type of code was written.
Such consumer code caused dead lock, producer could not add element because it is unable to get lock and most of the time lock was acquired by isEmpty function.
For such type of code thread dump is also interesting, you will see producer blocked but no trace of consumer thread. Multiple thread dump are required to confirm this.
Blocking queue should be used in why it is designed to , in case if you use non blocking calls then you must have some backoff strategy to avoid dead lock.
- Single consumer calling take in loop
This is very common pattern, message are taken from queue one by one, since blocking queue is lock based it needs lock for each operation.
It is always better to get multiple items from queue in one call by using drainTo type of function, this will reduce contention will give significant performance gain.
- size function for heart beat
This is also very common case where size is use to print heartbeat message. Size is not cheap operation for blocking queue, it needs lock to get size.
Atomic variable can be used to keep track of queue size and read to atomic variable will not have any effect on queue performance.
- Using sleep waiting strategy with ConcurrentLinkedQueue
Code using sleep waiting strategy
I am sure you will have very good reason to use CLQ, but sleep spoils every thing because CLQ does't have blocking support , so many application will use sleep to avoid spin loop.
Sleep based approach fine in some scenario, but if you really want to build event based system then sleep does't help,you need some kind of notification mechanism, so when ever element is added to queue it will notify consumer about that.
It is pretty easy to add such support to CLQ. I created blocking concurrent queue to try few things, executor-with-concurrentlinkedqueue blog has some details about it.
Conclusion
Each type of queue is designed for very specific use case and it should be used in proper way.
Some of the bad practice mention in blog makes application slow/unresponsive, bad usage pattern should be avoided.
