801961 - perhaps we shouldn't create #cores unused compilation threads

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

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?

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yes please!

Comment 2

[Mike Hommey [:glandium]]

(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.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Not a blocker. But if the JS team takes this one on we'd certainly appreciate it.

Comment 4

[:Benjamin Peterson]

Attachment: make compilation threads started lazily

Comment 5

[Luke Wagner [:luke]]

Someone beatcha to it.

Comment 6

[:Benjamin Peterson]

See bug 714050 comment 6, though.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Sorry, I guess I wasn't clear in bug 714050 comment 6. These two bugs complement each other and we want them both.

Comment 8

[Luke Wagner [:luke]]

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

Ah, duh, I wasn't thinking.  Nice fix here.

Comment 9

[:Benjamin Peterson]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d53bd74897b7

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/d53bd74897b7

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is this safe enough to take on aurora? Otherwise we'll still be spawning these threads in B2G.

Comment 12

[:Benjamin Peterson]

(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.

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

My understanding is that IonMonkey is disabled in B2G, so the threads will be disabled as well.

Comment 14

[:Benjamin Peterson]

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

Comment 15

[:Benjamin Peterson]

(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.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ah, then we might not care for B2G. But of course it might still be worth porting to aurora.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

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.