Last Comment Bug 672086 - Workers: Recycle threads to prevent churn and stack recursion problems
: Workers: Recycle threads to prevent churn and stack recursion problems
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-16 17:05 PDT by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2011-08-01 17:25 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, v1 (16.96 KB, patch)
2011-07-16 17:05 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
jonas: review+
Details | Diff | Splinter Review

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2011-07-16 17:05:54 PDT
Created attachment 546362 [details] [diff] [review]
Patch, v1

Workers currently create their own thread and then destroy it when they complete. If many workers all finish at the same time then multiple thread shutdown events will hit the main thread at the same time. Each of these events spins the event loop and so we can easily run out of stack space. Besides all that there's no reason not to hang on to finished threads and reassign them to new workers if possible.

Attached patch hangs on to a max of 20 finished (idle) threads for 30 seconds. They're reassigned in LIFO order to hopefully keep memory from paging.
Comment 1 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-07-22 10:32:09 PDT
Comment on attachment 546362 [details] [diff] [review]
Patch, v1

Review of attachment 546362 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 2 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-07-25 21:39:35 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/e385a9429c3a
Comment 3 Marco Bonardo [::mak] 2011-07-26 04:05:28 PDT
http://hg.mozilla.org/mozilla-central/rev/e385a9429c3a
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-31 19:41:24 PDT
Any particular reason to not use nsIThreadPool here?
Comment 5 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-08-01 08:25:56 PDT
(In reply to comment #4)
> Any particular reason to not use nsIThreadPool here?

Well, we want to allow a capped number of threads per domain, so we would have to have a threadpool per origin (threadpools will run anything in the queue, no way to filter currently). If we did that then free threads in one domain's pool couldn't be used by another domain that hadn't yet reached its limit since the pool manages all its own threads. The end result would be that it causes us to have more idle threads than we really want.

Is that argument compelling enough?
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-01 17:25:23 PDT
Definitely!

Note You need to log in before you can comment on or make changes to this bug.