Panorama stops working, if set javascript.options.methodjit.chrome to true

RESOLVED FIXED in Future

Status

()

defect
P2
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: alice0775, Assigned: dvander)

Tracking

({regression})

Trunk
Future
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101014 Firefox/4.0b8pre ID:20101014041748

Panorama stops working, if set javascript.options.methodjit.chrome to true

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile+ javascript.options.methodjit.chrome=true
2. Open Tabs
3. Open Panorama Ctrl + E

Actual Results:
 no tab in panorama


Regression window:
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3c762301b719&tochange=29c228a4d7eb
Confirmed.
Duplicate of this bug: 604389
I reported same bug for linux, and Ben for Mac, so OS -> All.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Blocks: 597043
It seems to have appeared between the 10/12 nightly and the 10/13 nightly, which should be this window:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2593c8c8af8b&tochange=f6e81dd5a125
Is this a JavaScript bug or a Tab Candy bug? Who should be working on it?
Duplicate of this bug: 605796
Do we know if we will be turning this setting (javascript.options.methodjit.chrome to true) on for Firefox 4. My understanding is that we aren't, which means this bug probably doesn't need to be fixed as a P1 or P2 for shipping.

CC'ing Dave for insight.
I'm not the Dave you're looking for. Luckily one of them (dmandelin) is CC'ed. CC'in another (dvander).

Dave #24601
I haven't heard of any plans to turn on chrome for the method JIT, but if there's a correctness issue, it's worth diagnosing since it could affect content too.
(In reply to comment #7)
> Do we know if we will be turning this setting
> (javascript.options.methodjit.chrome to true) on for Firefox 4. My
> understanding is that we aren't, which means this bug probably doesn't need to
> be fixed as a P1 or P2 for shipping.

We're still thinking about that, but leaning no per dvander in comment 9. As far as we can tell, turning on jits for chrome has no effect on subjective performance. The minus to turning it on is the extra bugs, like this one. The pluses are that bugs discovered that way probably affect content as well, and would improve the overall engine once fixed; and that some users will turn it on anyway, and we want things to work well for them too.

My personal opinion at this point is that the new jit is in a fairly early stage, and we have enough bugs to keep us busy on the content side. Once that stabilizes, whenever that is, may be the time to turn it on for chrome as well.
Thanks for the info. In that case, it sounds like this is not something we will look at or focus on for the Firefox 4 timeline.
Priority: -- → P2
Target Milestone: --- → Future
Making this not block our beta 8 bug. 

When we do feel the JIT is ready, it may very well make a speed difference for Panorama, as it's very JavaScript-heavy. Could be a good test-case for the performance improvement of the JIT in chrome.
No longer blocks: 597043
(In reply to comment #12)
> Making this not block our beta 8 bug. 
> 
> When we do feel the JIT is ready, it may very well make a speed difference for
> Panorama, as it's very JavaScript-heavy. Could be a good test-case for the
> performance improvement of the JIT in chrome.

Interesting. Feel free to try it out any time and report how it does--I know some of our testers run with methodjit on for chrome, and it seems to work for the most part.
Yeah, at least keep testing -- best case, the crashes go away. Less likely but could be a lifesaver if content hits the same bug, we get some crucial diagnostic information. Please leave any such skidmarks here if you get them.

/be
I use JM for chrome. I saw no improvement with Panorama compared to TM only. :(
Duplicate of this bug: 602205
blocking2.0: --- → ?
Duplicate of this bug: 611408
blocking2.0: ? → final+
Assignee: general → dvander
Status: NEW → ASSIGNED
Posted patch fixSplinter Review
This affects the web but it's pretty obscure - you need to have a constructor that, inside a catch block, returns an object other than |this|. In chrome it's easier because it can also trigger with "let".

The bug is that constructors weren't checking fp->rval before assuming an implicit return (RETRVAL, STOP) are returning undefined.
Attachment #494529 - Flags: review?(dmandelin)
Attachment #494529 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/mozilla-central/rev/44573d17ec8c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.