Closed Bug 714050 Opened 8 years ago Closed 7 years ago

Make JS helper threads optional

Categories

(Core :: JavaScript Engine, defect)

All
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: igor, Assigned: billm)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [js:p2:fx18])

Attachments

(4 files)

+++ 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.
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.
Depends on: 743619
There's also now a source-compressor thread, which we also would want workers to opt out of.
Whiteboard: [js:p2:fx18]
Blocks: 798849
Assignee: igor → wmccloskey
Summary: Make helper GC thread optional → Make JS helper threads optional
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)
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)
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..!
> 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 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+
err, JS_NO_HELPER_THREADS
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+
Attachment #672018 - Flags: review?(luke) → review+
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+
Duplicate of this bug: 801961
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.
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.