Enable chrome methodjit (and profiling) by default

RESOLVED FIXED in mozilla6

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

({mobile, perf})

Trunk
mozilla6
mobile, perf
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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 0 makes this sound like it would just be a bug-finding exercise.  Or do you hope to get speed-ups as well?
On mobile, at least, the chrome JIT speedup was noticeable.  I would expect some improvements on desktop as well.
(Assignee)

Comment 3

6 years ago
(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.
(Assignee)

Comment 4

6 years ago
http://tbpl.mozilla.org/?tree=MozillaTry&rev=0fc1ccce012e
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.
(Assignee)

Comment 6

6 years ago
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?
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.
(Assignee)

Comment 8

6 years ago
Rob, what rationale was used for enabling the tracejit for chrome back in bug 500304?
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)
(Assignee)

Comment 10

6 years ago
With the compiling heuristics used for mjit these days, the startup hit on mobile may not be as severe either.
(Assignee)

Comment 11

6 years ago
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
(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?
(Assignee)

Comment 13

6 years ago
That's what it looks like to me too. I would bet that most are within the noise.
(The other advantage is converging on common behaviour for both content and chrome, which I think is non-zero.)
(Assignee)

Comment 15

6 years ago
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.
(Assignee)

Comment 16

6 years ago
Obviously, the browser was restarted between tests.
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.
(Assignee)

Comment 18

6 years ago
Created attachment 524787 [details] [diff] [review]
Turn on methodjit and profiling for chrome

This has been working fine for me.
Assignee: general → ryanvm
Status: NEW → ASSIGNED
Attachment #524787 - Flags: review?(sayrer)

Updated

6 years ago
Duplicate of this bug: 649741
Attachment #524787 - Flags: review?(sayrer) → review+
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.
Attachment #524787 - Flags: approval-mozilla-aurora?
Keywords: mobile, perf

Comment 21

6 years ago
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)
Attachment #524787 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/tracemonkey/rev/3c3f44c79685

Thanks, Ryan!
Keywords: checkin-needed
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/3c3f44c79685
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Blocks: 652989

Updated

6 years ago
Summary: Enable chrome methodjit (and profiling?) by default → Enable chrome methodjit (and profiling) by default
Target Milestone: --- → mozilla6
Methodjitting chrome consumes *a lot* of memory.  Are we seeing noticeable perf improvements on desktop here?
See bug 671759 comment 0 for an example -- 30MB of mjit-code in the chrome compartment after browsing all day.
You need to log in before you can comment on or make changes to this bug.