Port |Bug 450983 - use leakThreshold for SeaMonkey testers| to Firefox

NEW
Unassigned

Status

()

Firefox
Build Config
10 years ago
9 years ago

People

(Reporter: sgautherie, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed1.9.1b2: Av1])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 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

Comment 1

10 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.
(Reporter)

Updated

10 years ago
Blocks: 465952
(Reporter)

Updated

10 years ago
Blocks: 462545
(Reporter)

Comment 2

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

(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)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #349211 - Flags: review?(ted.mielczarek)
Attachment #349211 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 349211 [details] [diff] [review]
(Av1) Reduce |browserChrome| to 75000 from Infinity
[Checkin: Comment 4]

Seems sensible.
(Reporter)

Updated

10 years ago
Attachment #349211 - Attachment description: (Av1) Reduce |browserChrome| to 75000 from Infinity → (Av1) Reduce |browserChrome| to 75000 from Infinity [Checkin: Comment 4]
(Reporter)

Comment 4

10 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
(Reporter)

Updated

10 years ago
Depends on: 466248
(Reporter)

Comment 5

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

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)
Attachment #352441 - Flags: review?(ted.mielczarek)
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?
Attachment #352441 - Flags: review?(ted.mielczarek) → review+
(Reporter)

Comment 7

10 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
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 → (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 7]
(Reporter)

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [c-n: Bv1 is baking for 1.9.1, depends on bug 463513 getting there first]

Comment 8

10 years ago
Had to back this out due to persistent orange. Please try again.
(Reporter)

Comment 9

10 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
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 7] → (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9]
(Reporter)

Comment 10

10 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)
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9] → (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9 & 10]
(Reporter)

Comment 11

10 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
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9 & 10] → (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9 & 11]
(Reporter)

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [c-n: Bv1 is baking for 1.9.1, depends on bug 463513 getting there first] → [(Av1 and) Bv1 is fixed1.9.1]
Serge: until further notice, you have a blanket r=me to lower this threshold as leaks are fixed.

Updated

10 years ago
No longer blocks: 465952
Depends on: 465952
(Reporter)

Updated

10 years ago
No longer blocks: 462545
Depends on: 462545
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.
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.
(Reporter)

Comment 15

10 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...
(Reporter)

Updated

10 years ago
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 [Checkin: Comment 9 & 11] → (Bv1) Reduce |browserChrome| to 74000 from 75000 [(m-c) Backout: Comment 13; (m-1.9.1) Checkin: Comment 11]
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.
(Reporter)

Comment 17

10 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.
(Reporter)

Updated

10 years ago
Attachment #352441 - Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 [(m-c) Backout: Comment 13; (m-1.9.1) Checkin: Comment 11] → (Bv1) Reduce |browserChrome| to 74000 from 75000 [Backout: Comment 13 & 16]
Attachment #352441 - Attachment is obsolete: true
(Reporter)

Comment 18

10 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)
}
(Reporter)

Updated

10 years ago
Depends on: 472677
(Reporter)

Updated

10 years ago
No longer depends on: 472677
(Reporter)

Updated

10 years ago
Depends on: 472677
(Reporter)

Updated

10 years ago
Depends on: 473802
(Reporter)

Updated

9 years ago
Blocks: 475085
(Reporter)

Updated

9 years ago
Depends on: 469581
(Reporter)

Comment 19

9 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...
(Reporter)

Comment 20

9 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))
}
Assignee: sgautherie.bz → nobody
Status: ASSIGNED → NEW
(Reporter)

Comment 21

9 years ago
Curretn situation is:
FF misses Chrome and Browser-Chrome,
FF and SM miss A11y, Reftest and Crashtest.
(Reporter)

Updated

9 years ago
Blocks: 495533
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?
(Reporter)

Comment 23

9 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.
(Reporter)

Updated

9 years ago
Whiteboard: [(Av1 and) Bv1 is fixed1.9.1] → [fixed1.9.1b2: Av1]
(Reporter)

Updated

9 years ago
No longer blocks: 495533
Depends on: 495533
(Reporter)

Updated

9 years ago
Depends on: 471647
You need to log in before you can comment on or make changes to this bug.