[rfc][icedtea-web] ResourceTracker Thread Pool Enhancement

[Andrew Azores]

On 10/01/2014 12:26 PM, Jie Kang wrote:
> Hello,
>
> This patch replaces the manual thread management system in Resource Tracker with a fixed thread pool using Java's Executor Service class.
>
> The functionality remains the same as before. In terms of testing, using the Performance Test [1], running the reproducers and performing manual testing has shown no regressions.
>
> Thoughts?
>
>
> [1] : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2014-August/028980.html
>
>
> Regards,
>

In addition to everything Jacob has already said, I'd also like to 
comment on the pool having always exactly 5 threads. I agree with his 
assessment that it would be nice to have some statistical data and some 
heuristic to use to determine a number of threads to use, but I think 
this is pretty infeasible. Really, the optimization problem here IMO is 
to try to minimize the number of threads used while maximizing network 
bandwidth utilization; in other words, there's no point downloading 5 at 
once if the first 3 would have already saturated the connection, but 
there's also no point downloading 5 if their combined utilization is 
only 50% of the available bandwidth. But really, it's worse to be using 
only 50% of the available bandwidth than to be downloading more 
resources than is required to saturate bandwidth, since the cost of 
extra threads is really not very high, but the cost to the user of 
downloading at less than full speed is (potentially a lot of) wasted 
time for no good reason. So, I think it's better to overshoot the 
optimal number of threads than to undershoot it. The cost of 
overshooting is just so minimal, which Jacob also says.

That said, it would be silly to pre-allocate some huge number of threads 
for this thread pool, say, 1000 threads. The chances of a user's applet 
having 1000 things for us to download is very small. If there are only 5 
resources then there are 995 threads for no reason whatsoever; worse, 
all 1000 threads will be sitting around in the JVM with nothing to do 
until the JVM is exited because the user is done with the applet. 
Although this probably isn't a massive performance issue, it's still 
wasteful.

So, with these thoughts in mind, I'd recommend you consider switching 
from newFixedThreadPool(5) to simply newCachedThreadPool [0]. It does 
mean that every resource will be downloaded at once, rather than queued 
up for processing, but AFAICT this will allow for a good amount of 
architectural simplification in ResourceTracker, and it also guarantees 
that if it's possible to saturate the download bandwidth, then we will, 
and so the task will be completed as quickly as possible (ignoring 
scheduler overhead etc). And, it also means that after the download 
tasks are completed, the additional threads will eventually be cleaned 
up automatically, unless additional tasks come in, in which case some of 
the threads will be reused. IMO this is pretty much exactly the best 
behaviour we can reasonably provide, unless we start gathering that 
statistical data and measuring maximum download throughput vs current 
resource download combined throughput, and all those extra details. To 
me all of that work probably isn't worth the effort - but it would 
definitely be very, very cool if it was done ;)

[0] 
http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html#newCachedThreadPool%28%29

Thanks,

-- 
Andrew Azores