Safe mode should disable JIT

VERIFIED FIXED

Status

()

VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: benjamin, Assigned: benjamin)

Tracking

(Depends on: 1 bug, {verified1.9.1})

Trunk
verified1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
Having safe-mode on should disable JIT. This will probably involve moving nsIXULRuntime around a bit so that it is available to the DOM code.
bug 342333 has a patch that might be useful for moving nsIXULRuntime
If this is a reasonably small amount of work, it might be block-worthy.  Nominating.
Flags: blocking1.9.1?
(Assignee)

Comment 3

10 years ago
Created attachment 356067 [details] [diff] [review]
Disable JIT when safe mode is on, rev. 1

Patch should work, haven't really tested it yet.
Attachment #356067 - Flags: superreview?(jst)
Attachment #356067 - Flags: review?(jst)
Comment on attachment 356067 [details] [diff] [review]
Disable JIT when safe mode is on, rev. 1

- In xpcom/system/Makefile.in

 XPIDLSRCS = \
         nsIXULAppInfo.idl \
+	nsIXULRuntime.idl \
         nsIGConfService.idl \

Fix the inconsistent indentation?

r+sr=jst
Attachment #356067 - Flags: superreview?(jst)
Attachment #356067 - Flags: superreview+
Attachment #356067 - Flags: review?(jst)
Attachment #356067 - Flags: review+
(Assignee)

Comment 5

10 years ago
http://hg.mozilla.org/mozilla-central/rev/3b9d2ef2e4a8 (with the wrong bug# in the commit message, argh!)
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Attachment #356067 - Flags: approval1.9.1?
I don't understand why this is useful. If someone is crashing due to content JIT, there are easier and more obvious workarounds than "start in safe mode to disable it".

Comment 7

10 years ago
This bug was a waste of time and added code bloat.  The time and code could have been better spent fixing 333808 which would have and will one day, have greater effect then this bug.  At least with JIT, it can be disabled through about:config unlike userChrome and userContent.
(In reply to comment #6)
> I don't understand why this is useful. If someone is crashing due to content
> JIT, there are easier and more obvious workarounds than "start in safe mode to
> disable it".

By that rationale safe mode is pointless since there are specific ways to disable everything it disables. The point is that safe mode is recommended as the first thing to try, at which point you can then start narrowing down whether it might be an extension problem, or JIT, or whatever...
(In reply to comment #8)
> By that rationale safe mode is pointless since there are specific ways to
> disable everything it disables.

That's not true - extensions and plugins can make the browser unusable enough that disabling them is not possible (i.e. a crash on startup). Content JIT crashes, on the other hand, can only be triggered by web pages that you visit, and we have protection against pages continuously crashing you on startup due to session restore (we avoid automatically restoring the session the second time around).

Comment 10

10 years ago
(In reply to comment #9)
> (In reply to comment #8)
> > By that rationale safe mode is pointless since there are specific ways to
> > disable everything it disables.
> 
> That's not true - extensions and plugins can make the browser unusable enough
> that disabling them is not possible (i.e. a crash on startup). Content JIT
> crashes, on the other hand, can only be triggered by web pages that you visit,
> and we have protection against pages continuously crashing you on startup due
> to session restore (we avoid automatically restoring the session the second
> time around).

Great point.  By that comment alone, that should be enough reason not to over code things needlessly.  Hell, we might as well just disable the whole javascript engine in safe mode if we are worried about crashes on webpages.  This bug just seems like a bandaid in that it is believed users are going to experience a load of crashes with JIT.  If that is the case, set JIT to FALSE until it is felt the code is stable enough for the default to be TRUE.

I still feel that fixing bug 333808 would have greater use.
Attachment #356067 - Flags: approval1.9.1? → approval1.9.1+
(In reply to comment #10)
> I still feel that fixing bug 333808 would have greater use.

It's actually worth noting that pretty much all of the changes that went into this patch would have been necessary in order to fix bug 333808 anyway. In fact now they are done fixing bug 333808 is not far off copying the safe mode check to the place I indicated in that bug.
> This bug was a waste of time and added code bloat.

Regardless of whether this is true, your tone seems rude to me.  Please keep it friendly.
Flags: blocking1.9.1?
This also makes it harder to figure out if there's a problem with an extension and whether or not it involves chrome jit. I.e. you can't use safe mode to quickly disable all add-ons -- the problem might be gone because jit is disabled, because an extension is disabled, or both.

For example, see the confusion in http://forums.mozillazine.org/viewtopic.php?p=5774025#p5774025 -- trying safe mode was entirely redundant there, as the problem was already known to be tied to javascript.options.jit.chrome=true. To see that there's no extension involved, the user had to manually disable all extensions.
Depends on: 478846
On Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090305 Minefield/3.2a1pre

Hi, is this fixed?  i'm trying to verify this, but i'm unsure if i'm seeing expected behavior or not testing properly.

STR: 
1) login to build above, set jit.content & jit.chrome = true
2) Save & Close browser, restart in safemode
3) open about:config, search on jit
4) jit.content 8* jit.chrome still shows = true.

Shouldn't i be expecting both to = false when opening in safemode?
(Assignee)

Comment 16

10 years ago
tchung, no: this doesn't alter the pref values at all: it only alters whether we read and apply those values, or just always disable JIT.
in that case, is there a way i can test this as a user?   Where is the testcase for this patch?
You can run a JS benchmark. For chrome jit, I guess you'll have to find a crash that depends on it.
(In reply to comment #18)
> You can run a JS benchmark. For chrome jit, I guess you'll have to find a crash
> that depends on it.

Dao, thanks for the tip.  Using the crashing URL testcase in bug 481302, i can verify that safemode does indeed disable jit.chrome.  

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090304 Firefox/3.1b3
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090305 Minefield/3.2a1pre
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.