Serge Gautherie (:sgautherie) Reporter
	Description • 16 years ago

Thunderbird does not run the 3 "mochitest" suites (yet).

SeaMonkey sets actual thresholds for each of the 3*3 cases.
Firefox should do the same.

So that bug 360482 comment 4
{
From  Serge Gautherie   2008-10-16 21:50:25 PDT

[...] maybe the default for BrowserChrome could be set now !?
(Someone would have to check what the current situation is.)

*****

Obviously, my wish would be that the default in the code is 0 for all (= no
more INFINITY default);
and that each "project" set its own defaults as low as possible !
(As SeaMonkey has now ;-))
}
could be done.

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py.in?mark=179,266-270,445-461#444

	Justin Wood (:Callek)
	Comment 1 • 16 years ago

My best suggestion to go about fixing this, is to utilize the leakThreshold= argument of buildbotcustom's init of these files... rather than passing in --leakThreshold to the python argument *directly* (and yes, fixing mochitest to always be INFINITY otherwise, **or** to pull in threshold numbers from a file in /app/mochi-thresholds.py or some such, but that is a deal for another day) ;-)

I am still not sure yet if the Failure the SeaMonkey boxes experienced when passing it in via that method was a fault in the code, or a fault in the setup.

	Serge Gautherie (:sgautherie) Reporter
	Comment 2 • 16 years ago

(In reply to comment #0)
> [...] maybe the default for BrowserChrome could be set now !?
> (Someone would have to check what the current situation is.)

There are 2 "remaining"/leaking bugs only;
it's 1.9.1b2 branch time;
the leaks (figures) are consistent on all 3 platforms;
this can't be too soon to start enforcing a threshold (of 75000 for now).

***

Current values from the 3*2 trunk tinderboxes:
TEST-PASS | runtests-leaks | WARNING leaked 73622 bytes during test execution (no threshold set)
TEST-PASS | runtests-leaks | WARNING leaked 74502 bytes during test execution (no threshold set)
TEST-PASS | runtests-leaks | WARNING leaked 71210 bytes during test execution (no threshold set)
TEST-PASS | runtests-leaks | WARNING leaked 71210 bytes during test execution (no threshold set)
TEST-PASS | runtests-leaks | WARNING leaked 71942 bytes during test execution (no threshold set)
TEST-PASS | runtests-leaks | WARNING leaked 72058 bytes during test execution (no threshold set)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/20245c2d97d0)

TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 71426 bytes during test execution (threshold set at 64 bytes)

	(not currently active) Ted Mielczarek
	Comment 3 • 16 years ago

Comment on attachment 349211 [details] [diff] [review]
(Av1) Reduce |browserChrome| to 75000 from Infinity
[Checkin: Comment 4]

Seems sensible.

	Serge Gautherie (:sgautherie) Reporter
	Comment 4 • 16 years ago

Comment on attachment 349211 [details] [diff] [review]
(Av1) Reduce |browserChrome| to 75000 from Infinity
[Checkin: Comment 4]

http://hg.mozilla.org/mozilla-central/rev/f93edfbd082e

	Serge Gautherie (:sgautherie) Reporter
	Comment 5 • 16 years ago

Account for fixed leak, yet leaving a little (more) fuzz.

After fixing bug 463513 (which fixed bug 462545),
last remaining leak, from the 3*2 trunk tinderboxes:
TEST-PASS | runtests-leaks | WARNING leaked 72886 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 72886 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 70470 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 70470 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71166 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71166 bytes during test execution (threshold set at 75000 bytes)

	(not currently active) Ted Mielczarek
	Comment 6 • 16 years ago

Comment on attachment 352441 [details] [diff] [review]
(Bv1) Reduce |browserChrome| to 74000 from 75000
[Backout: Comment 13 & 16]

From that log, looks like the highest it gets is 72886, do you want to just drop it to that (or 73000?) Or does it get higher in other instances?

	Serge Gautherie (:sgautherie) Reporter
	Comment 7 • 16 years ago

Comment on attachment 352441 [details] [diff] [review]
(Bv1) Reduce |browserChrome| to 74000 from 75000
[Backout: Comment 13 & 16]

(In reply to comment #6)

I don't know if the new figures are exactly stable or not:
I made a "guess" with 75000, which went well;
now, I prefer to reduce it by a safe amount (only) than get "false alerts".

***

http://hg.mozilla.org/mozilla-central/rev/a210c5c09190

	Robert Sayre
	Comment 8 • 16 years ago

Had to back this out due to persistent orange. Please try again.

	Serge Gautherie (:sgautherie) Reporter
	Comment 9 • 16 years ago

Comment on attachment 352441 [details] [diff] [review]
(Bv1) Reduce |browserChrome| to 74000 from 75000
[Backout: Comment 13 & 16]

(In reply to comment #8)
> Had to back this out due to persistent orange.

http://hg.mozilla.org/mozilla-central/rev/8406c99a7c89

I doubt this backout was needed...

***

> Please try again.

Anyway:

Current status:
TEST-PASS | runtests-leaks | WARNING leaked 73882 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 73882 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 70470 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 70470 bytes during test execution (threshold set at 74000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71174 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71166 bytes during test execution (threshold set at 75000 bytes)

Ted, ftr, that Linux "73882" is what I (lukily) noticed yesterday and why I prefered 74000 over 73500...

Here we go again:
http://hg.mozilla.org/mozilla-central/rev/e488ca8484ce

	Serge Gautherie (:sgautherie) Reporter
	Comment 10 • 16 years ago

Comment on attachment 352441 [details] [diff] [review]
(Bv1) Reduce |browserChrome| to 74000 from 75000
[Backout: Comment 13 & 16]

Current status on 1.9.1 branch:
TEST-PASS | runtests-leaks | WARNING leaked 73778 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 73894 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 70482 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71130 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71182 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71182 bytes during test execution (threshold set at 75000 bytes)

	Serge Gautherie (:sgautherie) Reporter
	Comment 11 • 16 years ago

Comment on attachment 352441 [details] [diff] [review]
(Bv1) Reduce |browserChrome| to 74000 from 75000
[Backout: Comment 13 & 16]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/554a5accf0ca

	(not currently active) Ted Mielczarek
	Comment 12 • 16 years ago

Serge: until further notice, you have a blanket r=me to lower this threshold as leaks are fixed.

	Justin Dolske [:Dolske]
	Comment 13 • 16 years ago

I've reset the leak threshold to 75,000 on 1.9.2, as the linux boxes have been orange for the past day with leaks just above the 74,000 threshold. dbaron took a look at the leaks, and noticed that we're still leaking the same things, but the size varies slightly. [Ie, we're not increasing this to hide new leaks]

Driving down the leak threshold is definitely something we should do, but it should only be dropped when the leaked contents change materially, and when the new threshold is high enough to not cause false alarms on tinderbox.

	Justin Dolske [:Dolske]
	Comment 14 • 16 years ago

Looking at 1.9.1, there's been one orange leak over the threshold (74,062 bytes) since this landed, and it was today. The threshold should probably be increased on 1.9.1 as well, although I'm inclined to leave it in for a bit longer to see if it's a once-a-week or once-a-month thing.

	Serge Gautherie (:sgautherie) Reporter
	Comment 15 • 16 years ago

(In reply to comment #13)
> should only be dropped when the leaked contents change materially, and when the

That's what I did here.

> new threshold is high enough to not cause false alarms on tinderbox.

Obviously, but I don't think I had any mean to predict it would increase (yesterday) or by what amount...

	Peter Van der Beken [:peterv]
	Comment 16 • 16 years ago

I just upped it to 75000 on 1.9.1 too, after checking in my patch for bug 461566 the linux tinderboxes went orange. There was a slight increase from my patch, but that's due to not shutting down XPConnect correctly because of existing leaks.

	Serge Gautherie (:sgautherie) Reporter
	Comment 17 • 16 years ago

http://hg.mozilla.org/mozilla-central/rev/b5d9ea223179
(Bv1) Reduce |browserChrome| to 72500 from 75000
[NB: This "Bv1" was a bad copy&paste :-<]

after bug 468160 comment 27, which figures are +/- the current ones.
NB: There may have been other increase/decrease since comment 13, but I didn't care to narrow them down.

	Serge Gautherie (:sgautherie) Reporter
	Comment 18 • 16 years ago

(In reply to comment #17)

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9587addcce39
Reduce |browserChrome| to 72500 from 75000

Current leaks are
{
TEST-PASS | runtests-leaks | WARNING leaked 71614 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71294 bytes during test execution (threshold set at 75000 bytes)
TEST-PASS | runtests-leaks | WARNING leaked 71990 bytes during test execution (threshold set at 75000 bytes)
}

	Serge Gautherie (:sgautherie) Reporter
	Comment 19 • 16 years ago

Bug 473845 reduced |browserChrome| to 0 from 72500 :-)

Yet, this bug should still be wanted:
*Mochitests could have new leaks detected (until fixed).
*Reftests will want this feature too. (See bug 469518)
*TUnit will want something related too. (See bug 469523)

NB: At this time, it looks like Thunderbird uses a "Bug 450983" like code for its leaks tinderboxes...

	Serge Gautherie (:sgautherie) Reporter
	Comment 20 • 16 years ago

Code that is currently there, but (now) useless:

http://mxr.mozilla.org/build/search?string=mochitest_leak_threshold
{
/buildbotcustom/process/factory.py
    * line 1746 -- mochitest_leak_threshold=None, **kwargs):
    * line 1881 -- leakThreshold=mochitest_leak_threshold,
    * line 1902 -- leakThreshold=mochitest_leak_threshold,
    * line 1921 -- leakThreshold=mochitest_leak_threshold,

/buildbot-configs/mozilla2-staging/master-main.cfg
    * line 186 -- mochitestLeakThreshold = pf.get('mochitest_leak_threshold', None)
    * line 314 -- mochitest_leak_threshold=mochitestLeakThreshold
}

http://mxr.mozilla.org/build/search?string=threshold
{
/buildbotcustom/unittest/steps.py
    * line 342 -- def __init__(self, leakThreshold=None, **kwargs):
    * line 345 -- if leakThreshold:
    * line 346 -- self.command.append("--leak-threshold=" + str(leakThreshold))
}

	Serge Gautherie (:sgautherie) Reporter
	Comment 21 • 15 years ago

Curretn situation is:
FF misses Chrome and Browser-Chrome,
FF and SM miss A11y, Reftest and Crashtest.

	David Baron :dbaron:
	Comment 22 • 15 years ago

What are you proposing to change here?  Aren't any leaks at all fatal in all of those test suites for Firefox?  Why do we need a threshold?

	Serge Gautherie (:sgautherie) Reporter
	Comment 23 • 15 years ago

(In reply to comment #22)
> What are you proposing to change here?

Extend threshold support.

> Why do we need a threshold?

For bugs like bug 493547 and bug 495533 (= bug 471647) for example.

	Kim Moir [:kmoir] ET
	Comment 24 • 6 years ago

triaging, moving bug to seamonkey not sure if this bug is still relevant

	Frank-Rainer Grahl (:frg)
	Comment 25 • 6 years ago

Fixed in SeaMonkey as far as I see it and was a log time ago so close it. Please reopen if you think otherwise.