Use SingleThreadExecutor for OrderedExecutor and drainTo() tasks into local array #3546
Conversation
|
Error: Failures: There are some failure tests related to this change. |
| return thread.getTaskCount(); | ||
| } | ||
| }); | ||
| if (thread instanceof ThreadPoolExecutor) { |
There was a problem hiding this comment.
The thread created by createSingleThreadExecutor is not a subclass of ThreadPoolExecutor, which means we will lose the following thread metrics.
There was a problem hiding this comment.
Good point. Also, I think there is an opportunity to add more stats around rejected & failed tasks. It would be very useful to see these in the charts.
There was a problem hiding this comment.
@hangc0276 Fixed, PTAL.
@zymap Fixed the test, PTAL |
|
Error: Failures: @merlimat Please take a look at the above-failed tests. |
| /** | ||
| * Unit test for {@link SingleThreadExecutor}. | ||
| */ | ||
| public class TestSingleThreadExecutor { |
There was a problem hiding this comment.
Can we add a test to validate the SingleThreadExecutor metrics?
There was a problem hiding this comment.
I've added assertions to verify the values of the counters. Do you mean to read the gauges out of the StatsLogger?
Motivation
OrderedExecutoris used extensively throughout the BK codebase and its performance is critical when there are many small entries.In the past we have experimented with different
BlockingQueuebackends inside the JDK defaultThreadPoolExecutorimplementation.The main problem with
ThreadPoolExecutoris that is not using thedrainTo()to avoid contention, because it's not making any assumptions on the queue type. If we assume an array based queue (likeArrayBlockingQueue), then thedrainTo()makes it much more efficient.A single executor with 16 thread submitting tasks can now process 46M tasks/s