1.2 MB leak regression on Tinderbox

VERIFIED FIXED

Status

SeaMonkey
General
P3
blocker
VERIFIED FIXED
18 years ago
13 years ago

People

(Reporter: dbaron, Assigned: Warren Harris)

Tracking

({mlk, regression, smoketest})

Trunk
x86
Linux
mlk, regression, smoketest

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+])

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
There was a 1.2 MB leak regression on the tinderbox leak stats late on August 9
or in the early morning hours of August 10.  It happened while the tree was red
and lots of people were checking in, so it's hard to point to the exact cause
from tinderbox.

The people who checked in between 23:14 and 00:29 were:
warren, morse, scc, valeski, mcafee, cmanske, alecf
[ some of the checkins couldn't have caused anything this bad, though ]

We now leak 15 of the 17 nsDocument objects created during the run, with 17
leaked references total.  We leak 1 of 3 GlobalWindowImpl, and 2 of 2
nsXULDocument.
(Reporter)

Updated

18 years ago
Keywords: mlk, smoketest

Comment 1

18 years ago
I was backing conrad's changes out.

Comment 2

18 years ago
What code in the editor, if any, is run for the leak tests?
I have one suspicious area in my checkin, but I doubt if it is run: it would 
be switching to "HTML Source" editmode, then back to "Normal" mode again.

Comment 3

18 years ago
over to warren for now.
Assignee: asa → warren
Keywords: nsbeta3
Whiteboard: [nsbeta3+]

Comment 4

18 years ago
isn't a textarea using editor code?

Comment 5

18 years ago
it's hard to tell, but there are 5 possible checkin blocks where the leak showed
up:

http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=965891700&maxdate=965892539
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=965891040&maxdate=965891699
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=965889660&maxdate=965891039
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=965888880&maxdate=965889659
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=965888040&maxdate=965888879

new leaks reported: attached

Comment 6

18 years ago
Created attachment 12676 [details]
new leaks
(Reporter)

Comment 7

18 years ago
The problem looks like warren's changes to the CSSLoader.  Testing fix now.

Comment 8

18 years ago
putting on blocker list to get some traction on this before
the tree opens.
Severity: normal → blocker
Keywords: regression
(Reporter)

Comment 9

18 years ago
Created attachment 12677 [details] [diff] [review]
proposed fix
(Assignee)

Comment 10

18 years ago
What I posted to porkjockeys last night... (I'm going to investigate today)...

Subject: 
         nsStringKey and hashtable changes
   Date: 
         Thu, 10 Aug 2000 02:07:18 -0700
   From: 
         Warren Harris <warren@netscape.com>
      To: 
         porkjockeys@mozilla.org




I checked in my changes tonight to optimize nsStringKey's memory
consumption. I split it into 2 classes nsStringKey and nsCStringKey, and
it turns out that nsCStringKey is what is usually called for in our
code. There are assertions in place if you try to mix different types of
keys in the same hashtable, so beware.

Making these changes seemed to reduce the tinderbox bloat figure by
about 1M. Looking at the log, I see almost all of this was due to nsStr
(as expected, nsStr 9628980  -5.36%). This is good.

However, at the same time I seem to have introduced a leak of about 43k
which I need to track down. Looks like keys _and_ hashtables are being
leaked (among other things). I have no idea why more hashtables would be
leaked than before, but I'm investigating...

Warren
(Reporter)

Comment 11

18 years ago
Your changes to nsCSSLoader turned a weak-reference-to-pointer hash key into a
strong-reference-to-pointer hash key, which created circular ownership between
document and CSS loader.  That's the 1.2M leak.

Comment 12

18 years ago
sounds like there's some traction on this. should we hold the tree closed today 
for this?
(Reporter)

Comment 13

18 years ago
Anybody want to review the fix?
(Assignee)

Comment 14

18 years ago
Fix looks good to me.
(Assignee)

Comment 15

18 years ago
And it's in... marking fixed.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 16

18 years ago
warren, david, can one of you verify this bug.  Thanks.
(Reporter)

Comment 17

18 years ago
OK.  Tinderbox currently shows 1-2K.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.