682326 - Disable method jit for chrome

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

When we turned on method jit for chrome, we didn't really see that much of a gain in performance, if any. But we are using more memory in the parent process because of it.

This patch turns off methodjit for chrome. You will still see some parent process mjit usage in about:memory because some webpage (content) are loaded in the parent process.

Comment 1

[Matt Brubeck (:mbrubeck)]

Comment on attachment 556061 [details] [diff] [review]
patch

Review of attachment 556061 [details] [diff] [review]:
-----------------------------------------------------------------

Don't check in the mozconfig.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

removed the mozconfig and actually set the pref to false
http://hg.mozilla.org/integration/mozilla-inbound/rev/60594ddf3d9e

Comment 3

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/60594ddf3d9e

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 556061 [details] [diff] [review]
patch

To help reduce memory usage

Comment 5

[Nicholas Nethercote [inactive]]

I second the aurora/beta nominations.  The method JIT uses a lot of memory.  And it should be very low-risk, since it was off previously.

Comment 6

[LegNeato]

Comment on attachment 556061 [details] [diff] [review]
patch

Approved for releases/mozilla-aurora and releases/mozilla-beta. Please land asap.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/b1d382004d3f
http://hg.mozilla.org/releases/mozilla-beta/rev/12bfaef14a35

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Is there a bug on file for doing this on desktop firefox?

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #8)
> Is there a bug on file for doing this on desktop firefox?

Not that I am aware of, but desktop might have different behavior/goals. It's a tradeoff and on mobile we traded for memory.

Comment 10

[Jesse Ruderman]

I'm worried this change will make computationally intensive add-ons much slower.

Why was this solution chosen as opposed to, say, tweaking the method jit heuristics to kick in less often?

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Are people running computationally intensive addons on mobile?

Also, there's still the tracer, right?

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #11)
> Are people running computationally intensive addons on mobile?

It's possible I guess

> Also, there's still the tracer, right?

Yes, the tracer is still enabled

Comment 13

[Jesse Ruderman]

The tracer is disabled by default.  Does disabling the method jit actually enable the tracer?  How long until the tracer is ripped out?

Comment 14

[u279076]

qa- as no QA fix verification is needed

Comment 15

[Brian Hackett [Laid off!]]

I think the mjit's problem here is in its technique for discarding jitcode.  Stock JM occasionally tries to discard jitcode during GC, but can only do so if the jitcode's compartment does not have active frames on the stack.  I noticed when fixing bug 688292 that when we GC there is almost always chrome code on the stack (at least on some workloads), which will cause mjit code to be discarded even slower.

In compartments with TI enabled (not chrome) this is different, and all jitcode is discarded on every GC even in compartments with frames on the stack.  I tried to do this for stock JM too in bug 685358, but ran into problems with JM+TM integration (weird corruption problems).  By removing the tracer or decoupling it from JM, that bug could reland.