Closed
Bug 714050
Opened 13 years ago
Closed 12 years ago
Make JS helper threads optional
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: igor, Assigned: billm)
References
(Blocks 1 open bug)
Details
(Whiteboard: [js:p2:fx18])
Attachments
(4 files)
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #713985 comment 6 +++
The GC helper thread should be optional so we avoid creating extra thread per each web worker JS runtime.
Also, you could reduce the default stack size of the gc helper thread.
Reporter | ||
Comment 2•13 years ago
|
||
Hopefully I have a working patch later this weak after finishing couple of patches related to our switch to single-thread runtime. Those also touches the helper thread and it is easier to have those land first.
There's also now a source-compressor thread, which we also would want workers to opt out of.
Updated•12 years ago
|
Whiteboard: [js:p2:fx18]
Assignee | ||
Updated•12 years ago
|
Assignee: igor → wmccloskey
Summary: Make helper GC thread optional → Make JS helper threads optional
Assignee | ||
Comment 4•12 years ago
|
||
Pretty simple refactoring. I considered giving this a default value and then having a function to change it. However, it's a lot simpler if it's immutable so that we don't have to worry about starting up threads or shutting them down when you change the value.
Attachment #672015 -
Flags: review?(luke)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #672016 -
Flags: review?(luke)
Assignee | ||
Comment 6•12 years ago
|
||
Note that this is separate from Benjamin's patch in bug 801961. This patch turns off the compile thread for non-main-thread runtimes. It also makes the whole thing work when there's only one core, which I think might have been broken before. It doesn't actually disable the compile thread. Benjamin's patch effectively does that.
Attachment #672018 -
Flags: review?(luke)
Assignee | ||
Comment 7•12 years ago
|
||
This means we won't be compression source for workers. This doesn't seem like a big deal to me, and it would be easy to compress in the foreground if we decide that would be useful.
Attachment #672019 -
Flags: review?(luke)
I lack the appropriate ASCII art skills to convey the dance I'm doing right now..!
Comment 9•12 years ago
|
||
> This means we won't be compression source for workers. This doesn't seem
> like a big deal to me, and it would be easy to compress in the foreground if
> we decide that would be useful.
Are threads inherently expensive, or is it just the stack space that's the problem? If it's the latter, we should just reduce the stack size and then we'd have more uniformity.
Comment 10•12 years ago
|
||
Comment on attachment 672015 [details] [diff] [review]
add a param to JS_NewRuntime to ask for helper threads, v1
Whoa, there are 2 JS_NewRuntime calls I never knew about...
One nit: can this be a named enum so that we can read:
JS_NewRuntime(..., JS_NO_BACKGROUND_FREE_THREAD);
?
Attachment #672015 -
Flags: review?(luke) → review+
Comment 11•12 years ago
|
||
err, JS_NO_HELPER_THREADS
Comment 12•12 years ago
|
||
Comment on attachment 672016 [details] [diff] [review]
make GC thread optional, v1
Nice for this not to be #ifdefs anymore.
Attachment #672016 -
Flags: review?(luke) → review+
Updated•12 years ago
|
Attachment #672018 -
Flags: review?(luke) → review+
Comment 13•12 years ago
|
||
Comment on attachment 672019 [details] [diff] [review]
make source compression thread optional, v1
Nice job breaking this up, thanks!
Attachment #672019 -
Flags: review?(luke) → review+
Assignee | ||
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3f603fdf6356
https://hg.mozilla.org/mozilla-central/rev/08302471419d
https://hg.mozilla.org/mozilla-central/rev/15791e9e6e5d
https://hg.mozilla.org/mozilla-central/rev/35b7bc10cc42
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment 17•12 years ago
|
||
So does this affect to GC pause times on workers?
If so, we may have to change the behavior again once we have all the
audio processing APIs in the workers.
Assignee | ||
Comment 18•12 years ago
|
||
Yes, this will affect GC pause times on workers. Incremental GC is also disabled on workers. To enable it, I think I'd have to add some extra write barriers.
Also, it's possible we could try to share one GC thread across multiple runtimes. That probably wouldn't be too big of a change.
I'd be really surprised if GC time on the worker changed much though; it has always been lightning fast.
I think this might be needed in order to get bug 813867 on mozilla-b2g18...
You need to log in
before you can comment on or make changes to this bug.
Description
•