646312 - Enable chrome methodjit (and profiling) by default

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

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 [inactive]]

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 emeritus)]

On mobile, at least, the chrome JIT speedup was noticeable.  I would expect some improvements on desktop as well.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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

http://tbpl.mozilla.org/?tree=MozillaTry&rev=0fc1ccce012e

Comment 5

[David Mandelin [:dmandelin]]

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]]

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 [inactive]]

Memory usage is of concern here!  Some about:memory numbers before and after would be good (js/mjit-code is the relevant number).  techcrunch.com is a good example workload.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Rob, what rationale was used for enabling the tracejit for chrome back in bug 500304?

Comment 9

[Mike Shaver (:shaver emeritus)]

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]]

With the compiling heuristics used for mjit these days, the startup hit on mobile may not be as severe either.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

FYI:
http://perf.snarkfest.net/compare-talos/index.html?oldRevs=422bbd8245a7&newRev=0fc1ccce012e&tests=a11y,tdhtml,tdhtml_nochrome,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tgfx,tgfx_nochrome,tscroll,tsvg,tsvg_opacity,ts,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen&submit=true

Comment 12

[David Mandelin [:dmandelin]]

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

That's what it looks like to me too. I would bet that most are within the noise.

Comment 14

[Mike Shaver (:shaver emeritus)]

(The other advantage is converging on common behaviour for both content and chrome, which I think is non-zero.)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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.

DISABLED
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

ENABLED
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]]

Obviously, the browser was restarted between tests.

Comment 17

[Nicholas Nethercote [inactive]]

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]]

Attachment: Turn on methodjit and profiling for chrome

This has been working fine for me.

Comment 20

[David Mandelin [:dmandelin]]

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

[LegNeato]

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]]

http://hg.mozilla.org/tracemonkey/rev/3c3f44c79685

Thanks, Ryan!

Comment 23

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/3c3f44c79685

Comment 24

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

Methodjitting chrome consumes *a lot* of memory.  Are we seeing noticeable perf improvements on desktop here?

Comment 25

[Nicholas Nethercote [inactive]]

See bug 671759 comment 0 for an example -- 30MB of mjit-code in the chrome compartment after browsing all day.