Last Comment Bug 646312 - Enable chrome methodjit (and profiling) by default
: Enable chrome methodjit (and profiling) by default
: mobile, perf
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla6
Assigned To: Ryan VanderMeulen [:RyanVM]
: Jason Orendorff [:jorendorff]
: 649741 (view as bug list)
Depends on:
Blocks: 652989
  Show dependency treegraph
Reported: 2011-03-29 19:03 PDT by Ryan VanderMeulen [:RyanVM]
Modified: 2011-07-14 21:58 PDT (History)
12 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Turn on methodjit and profiling for chrome (2.43 KB, patch)
2011-04-08 17:35 PDT, Ryan VanderMeulen [:RyanVM]
dmandelin: review+
christian: approval‑mozilla‑aurora-
Details | Diff | Splinter Review

Description Ryan VanderMeulen [:RyanVM] 2011-03-29 19:03:08 PDT
To date, only the tracejit is enabled by default for chrome. However, methodjit and profiling have hidden prefs currently set to false. We should try enabling these prefs to see what if any regressions they cause.
Comment 1 Nicholas Nethercote [:njn] 2011-03-30 03:26:26 PDT
Comment 0 makes this sound like it would just be a bug-finding exercise.  Or do you hope to get speed-ups as well?
Comment 2 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-03-30 12:09:07 PDT
On mobile, at least, the chrome JIT speedup was noticeable.  I would expect some improvements on desktop as well.
Comment 3 Ryan VanderMeulen [:RyanVM] 2011-03-30 14:40:46 PDT
(In reply to comment #1)
> Comment 0 makes this sound like it would just be a bug-finding exercise.  Or do
> you hope to get speed-ups as well?

Enabling the tracejit for chrome back in the day was a nice perf win, but it too was delayed due to concerns over regression risks. So I guess the thrust of comment #0 was mainly asking the question of finding out what if any regressions it causes so it can be turned on by default for the perf win.

Maybe this is Fx6 fodder so the switch can be made as early as possible in the development cycle? On the other hand, the test suite is considerably larger than it was in the tracejit-only days, so maybe even passing the current suite would be sufficiently confidence-inducing.
Comment 4 Ryan VanderMeulen [:RyanVM] 2011-03-30 14:59:58 PDT
Comment 5 David Mandelin [:dmandelin] 2011-03-30 16:25:21 PDT
David Anderson and I would like to see measurements that show that it actually helps. If so, then we can certainly flip the switch and see what happens.
Comment 6 Ryan VanderMeulen [:RyanVM] 2011-03-30 16:31:03 PDT
I'm running the full suite on the try push, so hopefully the numbers from that are sufficient. I have very little experience trying to compare try numbers to m-c, though...

Also, how much are the tests in the suite likely to benefit from chrome js speed improvements?
Comment 7 Nicholas Nethercote [:njn] 2011-03-30 17:06:14 PDT
Memory usage is of concern here!  Some about:memory numbers before and after would be good (js/mjit-code is the relevant number). is a good example workload.
Comment 8 Ryan VanderMeulen [:RyanVM] 2011-03-30 17:36:24 PDT
Rob, what rationale was used for enabling the tracejit for chrome back in bug 500304?
Comment 9 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-03-30 17:42:07 PDT
I wouldn't be surprised if the default Talos suite didn't show a change from this.  I would expect that we'd see benefits in things like tab close smoothness, maybe Tp from faster event listeners, possibly panorama layout.  Our chrome code is approximately of the same complexity as gmail, I expect, so there will be similar wins to be had.  Complex operations like web console filtering or Firebug wrangling may also show wins, as well as general responsiveness.  These things may be more noticeable on mobile, though IIRC when they turned it on they took a small startup time hit due to compilation overhead.

Note that we already JIT *component* code, since it doesn't have a chrome: URL prefix which is what the .chrome pref affects.

(my orphaned about:compartments patch will show mjit code size for the Firefox/chrome compartment, btw)
Comment 10 Ryan VanderMeulen [:RyanVM] 2011-03-30 17:44:18 PDT
With the compiling heuristics used for mjit these days, the startup hit on mobile may not be as severe either.
Comment 12 David Mandelin [:dmandelin] 2011-03-30 18:47:25 PDT
(In reply to comment #11)
> FYI:

Am I reading it right if it looks like there was a mix of slowdowns and speedups, and not a big change overall?
Comment 13 Ryan VanderMeulen [:RyanVM] 2011-03-30 18:49:48 PDT
That's what it looks like to me too. I would bet that most are within the noise.
Comment 14 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-03-30 18:59:57 PDT
(The other advantage is converging on common behaviour for both content and chrome, which I think is non-zero.)
Comment 15 Ryan VanderMeulen [:RyanVM] 2011-03-31 18:22:49 PDT
I created a session with GMail open as an app tab and this bug, TechCrunch, and about:memory open in other tabs. Once everything finished loading, I switched to about:memory and waited 5 minutes. After 5 minutes, I refreshed about:memory a couple times to make sure the numbers were stable and recorded them.

win32/privatebytes	258,375,680
win32/workingset	276,779,008
js/gc-heap		49,283,072
js/string-data		5,408,416
js/mjit-code		9,066,546

win32/privatebytes	267,890,688
win32/workingset	289,107,968
js/gc-heap		49,283,072
js/string-data		5,402,370
js/mjit-code		8,931,420

This is on my own 32bit build built with MSVC10 running on Win7 x64. No jemalloc, of course. I also have Adblock Plus 1.3.6rc.2955 enabled and active. None of my other extensions were running at the time (ChatZilla, FireFTP, etc).

Try run looked good, FWIW.
Comment 16 Ryan VanderMeulen [:RyanVM] 2011-03-31 18:23:59 PDT
Obviously, the browser was restarted between tests.
Comment 17 Nicholas Nethercote [:njn] 2011-03-31 20:27:33 PDT
Waiting 5 minutes isn't the best thing to do, because that's long enough that the method JIT might throw some old code away.  It'd be better to refresh about:memory as soon as everything has finished loading.

Having said that, it doesn't look like the extra memory usage is a problem.
Comment 18 Ryan VanderMeulen [:RyanVM] 2011-04-08 17:35:03 PDT
Created attachment 524787 [details] [diff] [review]
Turn on methodjit and profiling for chrome

This has been working fine for me.
Comment 19 Stuart Parmenter 2011-04-13 15:34:27 PDT
*** Bug 649741 has been marked as a duplicate of this bug. ***
Comment 20 David Mandelin [:dmandelin] 2011-04-14 11:39:27 PDT
Comment on attachment 524787 [details] [diff] [review]
Turn on methodjit and profiling for chrome

Requesting approval for landing to Aurora. Reasons to do this:

 - It is a pref change only.
 - It is trivial to back out.
 - It is expected to help mobile performance.
 - It will provide better test coverage for the methodjit.
Comment 21 christian 2011-04-15 13:18:12 PDT
Comment on attachment 524787 [details] [diff] [review]
Turn on methodjit and profiling for chrome

Denying for aurora:

1. Turning this on is arguably a feature and needs a QA plan, success criteria, etc
2. This isn't on m-c as far as we can tell (and probably project tracking regardless)
Comment 22 David Mandelin [:dmandelin] 2011-04-25 18:20:07 PDT

Thanks, Ryan!
Comment 23 Chris Leary [:cdleary] (not checking bugmail) 2011-04-26 15:39:16 PDT
Comment 24 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-14 21:57:12 PDT
Methodjitting chrome consumes *a lot* of memory.  Are we seeing noticeable perf improvements on desktop here?
Comment 25 Nicholas Nethercote [:njn] 2011-07-14 21:58:48 PDT
See bug 671759 comment 0 for an example -- 30MB of mjit-code in the chrome compartment after browsing all day.

Note You need to log in before you can comment on or make changes to this bug.