Closed Bug 617430 Opened 14 years ago Closed 14 years ago

Firefox 4 hangs when loading the AWS S3 Console


(Core :: JavaScript Engine, defect, P1)




Tracking Status
blocking2.0 --- betaN+


(Reporter: jhatax, Assigned: brendan)




(Keywords: hang, regression, Whiteboard: hardblocker, fixed-in-tracemonkey)


(4 files, 1 obsolete file)

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.
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.
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Attached file aws sample during hang
It's not necessary to sign up for S3, just AWS with an account:

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.
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.
blocking2.0: --- → ?
Ever confirmed: true
Keywords: regression
(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.

Bug 542002? Optimize to flat closures even if some upvars can't be copied
Depends on: 542002
Blocks: 542002
No longer depends on: 542002
I just tried with the latest nightly and the fix to optimize flat closures hasn't fixed the hang while loading the S3 Console.
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()) {
               if (CanFlattenUpvar(lexdep, funbox, tcflags)) {
               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
GWT, why do you hate me?

See also (independent) bug 501908.

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@!
Manoj: no sweat, it's our bug (mine) not GWT's -- just don't see that kind of code otherwise on the web.

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

Whiteboard: hardblocker
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
Fixing fields my automated script accidentally blanked. Apologies for the bugspam
Keywords: hang, regression
Blocks: 542002
Safe fix for b9.

Priority: -- → P1
Hardware: x86_64 → All
Target Milestone: --- → mozilla2.0b9
Need a code reviewer soon.

blocking2.0: betaN+ → ?
Attached 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.

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

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+

Whiteboard: hardblocker → hardblocker, fixed-in-tracemonkey
Flags: in-testsuite-
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.
Unless there is an hourly build that has the fix that I can test with right now (or within the next couple hours).

It seems to be have backed out for an unrelated orange? But that build avoids hanging for multiple seconds for me.
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!

Whiteboard: hardblocker, fixed-in-tracemonkey → hardblocker
Target Milestone: mozilla2.0b9 → mozilla2.0
Attached 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.

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+
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.


Whiteboard: hardblocker → hardblocker, fixed-in-tracemonkey
I picked up the latest tracemonkey build from here: 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.
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?

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