Note: There are a few cases of duplicates in user autocompletion which are being worked on.

quadratic behavior in JSScope::trace()/emptyShapeChange(), causing long gc pauses

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
JavaScript Engine
P2
critical
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: tnikkel, Assigned: brendan)

Tracking

({perf})

Trunk
mozilla2.0b7
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 449563 [details]
windbg

I run m-c nightlies and for the past few months I've been experiencing 20+ second GC pauses a few times a day. I did not used to experience this problem. I do not have any better idea of a regression range. My firefox.exe process usually uses between 500mb and 800mb, large, but not unreasonable. I've attached a windbg log with three backtraces during one such gc hang.

If it would be helpful I could try to bisect this problem using old nightlies, but it would be a long process as I only experience a few (about 1 to 5) of these pauses a day.

Updated

7 years ago
Severity: normal → major
Priority: -- → P2

Comment 1

7 years ago
   0  Id: 534.7b4 Suspend: 1 Teb: 7ffde000 Unfrozen
ChildEBP RetAddr  
0012c62c 002ce207 mozjs!js::PropertyTree::emptyShapeChange(unsigned long oldEmptyShape = 0x12d2d0, unsigned long newEmptyShape = 0xc7b02e0)+0x2e [e:\builds\moz2_slave\mozilla-central-win32-nightly\build\js\src\jspropertytree.cpp @ 407]
0012c668 0024175d mozjs!js_TraceObject+0x8cfa7

void
PropertyTree::emptyShapeChange(uint32 oldEmptyShape, uint32 newEmptyShape)
{
    if (oldEmptyShape == newEmptyShape)
        return;

    PropertyRootEntry *rent = (PropertyRootEntry *) hash.entryStore;
    PropertyRootEntry *rend = rent + JS_DHASH_TABLE_SIZE(&hash);

    while (rent < rend) {
        if (rent->emptyShape == oldEmptyShape)
            rent->newEmptyShape = newEmptyShape;
        rent++;
    }

    ++emptyShapeChanges;
}

Thanks Timothy. This is a very good bug report. Looks like we are running in circles in that function.

Updated

7 years ago
Attachment #449563 - Attachment mime type: application/octet-stream → text/plaon

Updated

7 years ago
Attachment #449563 - Attachment mime type: text/plaon → text/plain

Updated

7 years ago
blocking2.0: --- → ?
(Assignee)

Comment 2

7 years ago
Timothy, could you please try a tracemonkey nightly -- this may be a dup of bug 569391. The underlying pathological case will be removed by the patchwork for bug 558451. Thanks,

/be
(Reporter)

Comment 3

7 years ago
Ok, running on tracemonkey nightlies now.
(Reporter)

Comment 4

7 years ago
I just got another long pause using tracemonkey nightly 2010-06-07 (http://hg.mozilla.org/tracemonkey/rev/044852a34c7b).
(Assignee)

Comment 5

7 years ago
Timothy, can you describe what you have in the way of add-ons, loaded pages, and things like GreaseMonkey scripts? It would be great to reproduce this.

I'll fix bug 558451 this week, but now I'm concerned that won't make some deeper bug go away. It will get rid of the need to renumber empty scopes' shapes.

/be
(Reporter)

Comment 6

7 years ago
For addons I have Download Timestamp, FaviconizeTab, DOM Inspector, Uppity, Nightly Tester Tools, Modify Headers, Java Quick Starter (disabled), and Adblock Plus.

For pages loaded I have gmail, a bunch of bugzilla bug pages, google groups threads, google searches, wikipedia pages, and a bunch of random articles/blog posts that I will probably never get to. I can send a list of all my open tabs, but I obviously do not want to post that publicly.
(Assignee)

Comment 7

7 years ago
Timothy, if you are game could you try running for a while with add-ons disabled? If you can't live without some, disable half (group them in a list so we can try bisecting if there's a "wolf in the fold"). Thanks again,

/be
(Assignee)

Updated

7 years ago
Assignee: general → brendan
Depends on: 558451
(Reporter)

Comment 8

7 years ago
Ok, I kept the three that I need most: Download Timestamp, FaviconizeTab, and Adblock Plus. If I get another pause I'll disable them individually to see if I can pin it on one of them.
(Reporter)

Comment 9

7 years ago
Still running the same tracemonkey build (2010-06-07 nightly) I've gotten the long pauses with Download Timestamp, FaviconizeTab, and Adblock Plus enabled. And I've gotten long pauses with each one disabled individually. So it doesn't look to be caused by any specific addon.

Let me know if there is anything else you'd like me to try.
(Assignee)

Comment 10

7 years ago
Timothy -- thanks, no need to do more. Ball's in my court.

/be

Comment 11

7 years ago
dbaron confirmed that this is the likely cause for his 30s+ GC delays as well.

This is a must fix for FF4 IMO. Please block on this.

Updated

7 years ago
Severity: major → blocker

Updated

7 years ago
OS: Windows 2000 → All
Hardware: x86 → All
Summary: long (20+ seconds) gc pauses → quadratic behavior in JSScope::trace()/emptyShapeChange(), causing long gc pauses
(Assignee)

Comment 12

7 years ago
Created attachment 455927 [details] [diff] [review]
empty scope class counting instrumentation support, #ifdef DEBUG

This is a patch against m-c tip (applies to tm tip right now with --fuzz=7 needed for jscntxt.h).

David, anyone willing to apply and run with JS_EMPTYCLASS_DUMPFILE=/tmp/ec or some such, please attach non-empty /tmp/ec.N (N is the GC run number) files that seem associated with overlong GC runtime here.

I'll build and run m-c and try to do the same.

/be
(Assignee)

Comment 13

7 years ago
Created attachment 455929 [details] [diff] [review]
empty scope class counting instrumentation support, #ifdef DEBUG
Attachment #455927 - Attachment is obsolete: true
(Assignee)

Comment 14

7 years ago
Created attachment 455932 [details] [diff] [review]
empty scope class counting instrumentation support, #ifdef DEBUG

Don't generate a dumpfile if the emptyScopeClasses hashmap is empty.

Previous patch reversed dumpfile field on line order to put count first, for easier sort -nr, etc.

/be
Attachment #455929 - Attachment is obsolete: true

Updated

7 years ago
blocking2.0: ? → final+
Created attachment 458219 [details] [diff] [review]
empty scope class counting instrumentation support, #ifdef DEBUG

The previous patch was missing initialization of the hashtable.
Attachment #455932 - Attachment is obsolete: true
Keywords: perf
Created attachment 459625 [details]
gdb log

The revised patch still crashed, brendan says because of dangling JSClass pointers.

However, we enumerated a part of the hashtable (which was quite large) using gdb after the crash.

The live classes in the table were mostly the result of XBL.  nsXBLBinding::DoInitJSClass puts the address of the prototype in the JSClass, so there are a large number of unique classes.

Here's the full gdb output of our debugging session.
(Assignee)

Comment 17

7 years ago
This is all fixed in my patch queue now for bug 558451. The code in question is gone, and good riddance!

/be
Severity: blocker → critical
(Assignee)

Comment 18

7 years ago
Patch stack for bug 558451 landed on tm.

/be
Whiteboard: fixed-in-tracemonkey

Comment 19

7 years ago
be: is the intention to merge that tracemonkey branch change and fix this bug for 2.0?
(Assignee)

Comment 20

7 years ago
(In reply to comment #19)
> be: is the intention to merge that tracemonkey branch change and fix this bug
> for 2.0?

Of course -- lots of stuff still to do for Mozilla 2, why wouldn't we fix this (and all of bug 558451)? Merge from tm to m-c may happen this week, even.

/be

Comment 21

7 years ago
coming from my other gui-stall bugreport 

https://bugzilla.mozilla.org/show_bug.cgi?id=579653#c11

i was pointed here regarding my comment #11 over there, as that seems to belong rather here someone more experienced said.


my original bugreport was also about complete gui freezes in firefox4 betas every now and then and huge loads of various internal xul.dll functions and whatnot.


if it helps please look at the comment #11
https://bugzilla.mozilla.org/show_bug.cgi?id=579653#c11
for further information.

this happens with both beta4 and beta5candidatebuild1.
thanks and regards.
(Assignee)

Comment 22

7 years ago
See bug 558451 comment 166.

/be
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Target Milestone: --- → mozilla2.0b6
You need to log in before you can comment on or make changes to this bug.