Details
-
Enhancement
-
Resolution: Unresolved
-
P4
-
None
-
11, 17, 21, 22, 23
Description
We have seen a nice pitfall in some of our services. The highly parallel worker threads reported the metrics for another TPE, asking its getPoolSize(), getActiveCount(), etc. methods. Which lead to a severe contention on `TPE.mainLock`, since all those methods take it:
```
/**
* Returns the approximate number of threads that are actively
* executing tasks.
*
* @return the number of threads
*/
public int getActiveCount() {
final ReentrantLock mainLock = this.mainLock;
mainLock.lock();
try {
int n = 0;
for (Worker w : workers)
if (w.isLocked())
++n;
return n;
} finally {
mainLock.unlock();
}
}
```
This contention mode is rather unusual, since one would expect a single monitoring thread to access these methods, but it might not be as easily fixable on application side.
That lock seems to protect the concurrent access to `HashSet workers`. For stable thread pools -- where workers are rarely created, interrupted, or shut down -- we rarely modify it. Maybe we should instead use a ReentrantReadWriteLock here to alleviate this pitfall.
```
/**
* Returns the approximate number of threads that are actively
* executing tasks.
*
* @return the number of threads
*/
public int getActiveCount() {
final ReentrantLock mainLock = this.mainLock;
mainLock.lock();
try {
int n = 0;
for (Worker w : workers)
if (w.isLocked())
++n;
return n;
} finally {
mainLock.unlock();
}
}
```
This contention mode is rather unusual, since one would expect a single monitoring thread to access these methods, but it might not be as easily fixable on application side.
That lock seems to protect the concurrent access to `HashSet workers`. For stable thread pools -- where workers are rarely created, interrupted, or shut down -- we rarely modify it. Maybe we should instead use a ReentrantReadWriteLock here to alleviate this pitfall.