franz1981 commented on a change in pull request #2514: ARTEMIS-2236 Address
Latency Impact caused by ARTEMIS-1451
URL: https://github.com/apache/activemq-artemis/pull/2514#discussion_r250124916
##########
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadPoolExecutor.java
##########
@@ -64,20 +63,15 @@ public void setExecutor(ActiveMQThreadPoolExecutor
executor) {
public boolean offer(Runnable runnable) {
boolean retval = false;
- // Need to lock for 2 reasons:
- // 1. to safely handle poll timeouts
- // 2. to protect the delta from parallel updates
- synchronized (lock) {
- if ((executor.getPoolSize() >= executor.getMaximumPoolSize()) ||
(threadTaskDelta > 0)) {
- // A new task will be added to the queue if the maximum number
of threads has been reached
- // or if the delta is > 0, which means that there are enough
idle threads.
+ if (threadTaskDelta > 0 || (executor.getPoolSize() >=
executor.getMaximumPoolSize())) {
Review comment:
Not a significan change, but why you've inverted the checks here?
The common path should be that you have all the threads busy up and running
(so `executor.getPoolSize() >= executor.getMaximumPoolSize() = true`): it would
avoid checking the delta...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
