perhaps we shouldn't create #cores unused compilation threads

RESOLVED FIXED in mozilla19

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: luke, Assigned: Benjamin)

Tracking

unspecified
mozilla19
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:-, firefox18 affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
As bent pointed out the other day, we currently create # cores HW threads in JS.  These were added as part of bug 774253, but background Ion-compilation feature is hard-disabled atm so they are completely unused.  Eventually we may enable this feature, but for now it's just wasting resources.  Maybe it doesn't matter for desktop, but it seems like this could matter for b2g?
Yes please!
blocking-basecamp: --- → ?
Blocks: 798849

Updated

6 years ago
status-firefox18: --- → affected
(In reply to Luke Wagner [:luke] from comment #0)
> As bent pointed out the other day, we currently create # cores HW threads in
> JS.  These were added as part of bug 774253, but background Ion-compilation
> feature is hard-disabled atm so they are completely unused.  Eventually we
> may enable this feature, but for now it's just wasting resources.  Maybe it
> doesn't matter for desktop, but it seems like this could matter for b2g?

If they are unused, they are just wasting address space, not memory. I doubt this really matters for b2g: OOM will happen way before address space is exhausted.
Not a blocker. But if the JS team takes this one on we'd certainly appreciate it.
blocking-basecamp: ? → -
(Assignee)

Comment 4

6 years ago
Created attachment 671992 [details] [diff] [review]
make compilation threads started lazily
Attachment #671992 - Flags: review?(luke)
(Reporter)

Comment 5

6 years ago
Someone beatcha to it.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 714050
(Assignee)

Comment 6

6 years ago
See bug 714050 comment 6, though.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Sorry, I guess I wasn't clear in bug 714050 comment 6. These two bugs complement each other and we want them both.
(Reporter)

Comment 8

6 years ago
Comment on attachment 671992 [details] [diff] [review]
make compilation threads started lazily

Ah, duh, I wasn't thinking.  Nice fix here.
Attachment #671992 - Flags: review?(luke) → review+

Comment 10

6 years ago
https://hg.mozilla.org/mozilla-central/rev/d53bd74897b7
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Is this safe enough to take on aurora? Otherwise we'll still be spawning these threads in B2G.
(Assignee)

Comment 12

6 years ago
(In reply to Jonas Sicking (:sicking) from comment #11)
> Is this safe enough to take on aurora? Otherwise we'll still be spawning
> these threads in B2G.

Yes, it only affects a disabled-by-default feature.
My understanding is that IonMonkey is disabled in B2G, so the threads will be disabled as well.
(Assignee)

Comment 14

6 years ago
Comment on attachment 671992 [details] [diff] [review]
make compilation threads started lazily

[Approval Request Comment]
User impact if declined: Pointless spawning of threads
Testing completed (on m-c, etc.): On m-c
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None
Attachment #671992 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 15

6 years ago
(In reply to Bill McCloskey (:billm) from comment #13)
> My understanding is that IonMonkey is disabled in B2G, so the threads will
> be disabled as well.

Right, this doesn't matter if JS_ION is not defined.
Ah, then we might not care for B2G. But of course it might still be worth porting to aurora.
Comment on attachment 671992 [details] [diff] [review]
make compilation threads started lazily

If this isn't needed for uplift then we might as well let it ride the trains.
Attachment #671992 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-

Updated

6 years ago
Assignee: general → benjamin
You need to log in before you can comment on or make changes to this bug.