Firefox 4 hangs when loading the AWS S3 Console

RESOLVED FIXED in mozilla2.0

Status

()

defect
P1
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jhatax, Assigned: brendan)

Tracking

({hang, regression})

Trunk
mozilla2.0
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: hardblocker, fixed-in-tracemonkey, )

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_5; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.215 Safari/534.10
Build Identifier: 

Load the AWS S3 Console.
Log in with the AWS credentials for an account that has signed up for S3.
Browser hangs for maybe 20 seconds while loading the console.

IE9, Chrome, Firefox 3.6.x don't exhibit this behavior.

Reproducible: Always

Actual Results:  
Firefox hangs, but eventually loads the page after a long delay.

Expected Results:  
Firefox shouldn't hang. It should load the page in about the same time as Chrome 8.

Updated

9 years ago
Keywords: hang
Version: unspecified → Trunk
(In reply to comment #0)
> Log in with the AWS credentials for an account that has signed up for S3.

This makes it nearly impossible to process with your Issue unless you can attach a reduced Testcase and/or a saved copy of the Site showing your Issue.

Updated

9 years ago
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general

Comment 2

9 years ago
It's not necessary to sign up for S3, just AWS with an Amazon.com account:
https://console.aws.amazon.com/s3/home

It'll start loading then hang then show a message like "It looks like you don't have an Amazon S3 account."

The activity monitor sample ends with..

js::Compiler::compileScript(JSContext*, JSObject*, JSStackFrame*, ..
  js::Parser::setFunctionKinds(JSFunctionBox*, unsigned int&)
    js::Parser::setFunctionKinds(JSFunctionBox*, unsigned int&)

It hangs with methodjit/tracejit on/off.

Comment 3

9 years ago
This is just one of the javascript files used by AWS console. It's about 1.5MB.

If I make a dummy html file to load it, it'll hang:

<script src="aws.js"></script>

There's 8762 instances of "function" across 8698 lines. The first ~1600 lines seem to be creating empty constructors that get their prototypes set later.

Updated

9 years ago
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Keywords: regression
Reporter

Comment 4

9 years ago
(In reply to comment #1)
> (In reply to comment #0)
> > Log in with the AWS credentials for an account that has signed up for S3.
> 
> This makes it nearly impossible to process with your Issue unless you can
> attach a reduced Testcase and/or a saved copy of the Site showing your Issue.

Am sorry I didn't get an email notifying me of your response or else I would have responded earlier with information about a free-tier account. As Edward has pointed out though, an S3-enabled AWS account isn't necessary to reproduce this behavior.

Please let me know if the Javascript file in comment #3 doesn't contain sufficient information to track down the cause of this regression. I have added myself to the cc: list so that I am notified and can respond to your questions in a timely fashion.
blocking2.0: ? → beta9+
A regression window might help here.

Comment 7

9 years ago
http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=adaf878a0fbc&tochange=a8d65f5da19a

Bug 542002? Optimize to flat closures even if some upvars can't be copied
Depends on: 542002
Blocks: 542002
No longer depends on: 542002
Reporter

Comment 8

9 years ago
I just tried with the latest nightly and the fix to optimize flat closures hasn't fixed the hang while loading the S3 Console.
Reporter

Comment 9

9 years ago
I got this to repro on my Windows Vista and Windows 7 Minefield installations as well (both 32 and 64bit). Changing platform to All.
OS: Mac OS X → All
Almost all the time is spent within DeoptimizeUsesWithin, called from setFunctionKinds. The key part of setFunctionKinds is this:

    } else {
        // (Point 1) (referred to below)
        uintN nupvars = 0, nflattened = 0;
        /*
         * For each lexical dependency from this closure to an outer
         * binding, analyze whether it is safe to copy the binding's
         * value into a flat closure slot when the closure is formed.
         */
        while ((ale = iter()) != NULL) {
           JSDefinition *lexdep = ALE_DEFN(ale)->resolve();
           if (!lexdep->isFreeVar()) {
               ++nupvars;
               if (CanFlattenUpvar(lexdep, funbox, tcflags)) {
                   ++nflattened;
                   continue;
               }
               if (DeoptimizeUsesWithin(lexdep, funbox->node->pn_body->pn_pos)) {
                   FlagHeavyweights(lexdep, funbox, tcflags);
               }
            }
        }
    }


The test case defines about 7000 functions inside another function. Those functions share a bunch of upvars, which are defined on the last line of the outer function. If I delete that last line, parse time is not too bad. Otherwise, the last call (the last call entered, that is--it's recursive) to setFunctionKinds takes 10 seconds.

Some stats:

  calls to setFunctionKinds =               22
  runs of (Point 1) =                    6,519   (~ 1/function)
  calls to DeoptimizeUsesWithin =       71,984
  runs of Deopt loop body =        360,998,133

I don't know what list DeoptimizeUsesWithin is looping over, but it's doing about 5000 iterations per call. That would seem to be the source of the blowup.
Assignee: general → brendan
Assignee

Comment 11

9 years ago
GWT, why do you hate me?

See also (independent) bug 501908.

/be
Status: NEW → ASSIGNED
Reporter

Comment 12

9 years ago
Hey Brendan,

I didn't want to state outright that ours is a GWT application in the hope that this wasn't an inefficiency caused by GWT. Since filing this report though I've read posts about GWT compiled code wreaking havoc on the browser's Javascript engine. It looks like bug 501908 has a patch that could potentially fix this bug as well. I'll track both bugs to test a nightly with the fix.

Awesome work be@ and jimb@!
Assignee

Comment 13

9 years ago
Manoj: no sweat, it's our bug (mine) not GWT's -- just don't see that kind of code otherwise on the web.

/be
Assignee

Comment 14

9 years ago
(In reply to comment #12)
> It looks like bug 501908 has a patch that could potentially fix this
> bug as well. I'll track both bugs to test a nightly with the fix.

But as noted in comment 11 by "(independent)", that fix won't help here, AFAICS. Let me know if it does!

/be
Whiteboard: hardblocker

Comment 15

9 years ago
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
No longer blocks: 542002
blocking2.0: beta9+ → betaN+
Keywords: hang, regression

Comment 16

9 years ago
Fixing fields my automated script accidentally blanked. Apologies for the bugspam
Keywords: hang, regression

Updated

9 years ago
Blocks: 542002
Assignee

Comment 17

9 years ago
Safe fix for b9.

/be
Priority: -- → P1
Hardware: x86_64 → All
Target Milestone: --- → mozilla2.0b9
Assignee

Comment 18

9 years ago
Need a code reviewer soon.

/be
blocking2.0: betaN+ → ?
Assignee

Comment 19

9 years ago
Posted patch fix (obsolete) — Splinter Review
Not sure how to test this. Time-based tests are no good, big-O likewise in our bitter experience. Could instrument via an ad-hoc shell command but that seems too narrow. I say this is tested by inspecting the patch in light of dmandelin's profiling results in the bug.

/be
Attachment #502325 - Flags: review?(dmandelin)
Comment on attachment 502325 [details] [diff] [review]
fix

Yes, it does seem hard to test case, in the sense that all the test case ideas I can think of would be more trouble than they are worth. The patch clearly does eliminate the possibility of the blowup I documented in the initial analysis.
Attachment #502325 - Flags: review?(dmandelin) → review+
Seems important, safe fix, so it seems like a good blocker.
blocking2.0: ? → betaN+
Assignee

Comment 22

9 years ago
http://hg.mozilla.org/tracemonkey/rev/473344ac82e3

/be
Whiteboard: hardblocker → hardblocker, fixed-in-tracemonkey
Assignee

Updated

9 years ago
Flags: in-testsuite-
Reporter

Comment 23

9 years ago
I can test the next nightly that contains the fix with the AWS S3 Console to at least let you know whether the hang has gone away.
Reporter

Comment 24

9 years ago
Unless there is an hourly build that has the fix that I can test with right now (or within the next couple hours).

Comment 25

9 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/tracemonkey-macosx64/1294700426/firefox-4.0b9pre.en-US.mac64.dmg

It seems to be have backed out for an unrelated orange? But that build avoids hanging for multiple seconds for me.
Assignee

Comment 26

9 years ago
Mardak: yes, I fixed this bug but then backed out cuz Philor (whom one must never doubt because he is always right about orange, and has uncanny "Concentration" skills) blamed this patch for XPCOM leaks. Mysterious, but seemingly a fair cop!

/be
Assignee

Updated

9 years ago
Whiteboard: hardblocker, fixed-in-tracemonkey → hardblocker
Target Milestone: mozilla2.0b9 → mozilla2.0
Assignee

Comment 27

9 years ago
Posted patch fixed fixSplinter Review
Fixed patch. I am a bear of little brain lately. FUN_INTERPRETED(fun), tested by fun->isInterprted(), is not the same as FUN_KIND(fun) == JSFUN_INTERPRETED.

/be
Attachment #502325 - Attachment is obsolete: true
Attachment #502736 - Flags: review?(dmandelin)
Comment on attachment 502736 [details] [diff] [review]
fixed fix

Guess I should have checked that too. I just figured it was all right based on the names.
Attachment #502736 - Flags: review?(dmandelin) → review+
Assignee

Comment 29

9 years ago
I know, names are not hanging together well. I didn't review isInterpreted, and it is not just different from other predicates in using "isFoo" instead of "foo" style, it obviously is confusing w.r.t. FUN_KIND(fun) == JSFUN_INTERPRETED. I'll file a followup.

/be
Assignee

Comment 30

9 years ago
http://hg.mozilla.org/tracemonkey/rev/63d4e88f112f

/be
Whiteboard: hardblocker → hardblocker, fixed-in-tracemonkey
Reporter

Comment 31

9 years ago
I picked up the latest tracemonkey build from here: https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/tracemonkey-macosx64/1294773367/firefox-4.0b9pre.en-US.mac64.dmg. Loading the AWS S3 Console in this build doesn't hang the browser.

Is there a chance that this fix will make it into Beta9? Thanks for the fix @be.
Assignee

Comment 32

9 years ago
It missed the b9 freeze so it'll be in b10 for sure. We're still debating what to deliver in b9 for best effect, specifically user relief from bugs that have stunk up the place for too many betas, like this one.

Rob, any thoughts?

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