Closed Bug 672086 Opened 10 years ago Closed 10 years ago

Workers: Recycle threads to prevent churn and stack recursion problems

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Details

(Whiteboard: [inbound])

Attachments

(1 file)

Attached patch Patch, v1Splinter Review
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.
Attachment #546362 - Flags: review?(jonas)
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Comment on attachment 546362 [details] [diff] [review]
Patch, v1

Review of attachment 546362 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #546362 - Flags: review?(jonas) → review+
http://hg.mozilla.org/mozilla-central/rev/e385a9429c3a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Any particular reason to not use nsIThreadPool here?
(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?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.