Closed
Bug 460548
Opened 16 years ago
Closed 6 years ago
Port |Bug 450983 - use leakThreshold for SeaMonkey testers| to Firefox
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: sgautherie, Unassigned)
References
Details
(Whiteboard: [fixed1.9.1b2: Av1])
Attachments
(1 file, 1 obsolete file)
1.17 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
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•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.
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)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #349211 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #349211 -
Flags: review?(ted.mielczarek) → review+
Comment 3•16 years ago
|
||
Comment on attachment 349211 [details] [diff] [review] (Av1) Reduce |browserChrome| to 75000 from Infinity [Checkin: Comment 4] Seems sensible.
Reporter | ||
Updated•16 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•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
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)
Attachment #352441 -
Flags: review?(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?
Attachment #352441 -
Flags: review?(ted.mielczarek) → review+
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
Attachment #352441 -
Attachment description: (Bv1) Reduce |browserChrome| to 74000 from 75000 → (Bv1) Reduce |browserChrome| to 74000 from 75000
[Checkin: Comment 7]
Reporter | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [c-n: Bv1 is baking for 1.9.1, depends on bug 463513 getting there first]
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
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•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)
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•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
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•16 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]
Comment 12•16 years ago
|
||
Serge: until further notice, you have a blanket r=me to lower this threshold as leaks are fixed.
Updated•16 years ago
|
Reporter | ||
Updated•16 years ago
|
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.
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.
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...
Reporter | ||
Updated•16 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]
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.
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.
Reporter | ||
Updated•16 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•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) }
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...
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)) }
Assignee: sgautherie.bz → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 21•16 years ago
|
||
Curretn situation is: FF misses Chrome and Browser-Chrome, FF and SM miss A11y, Reftest and Crashtest.
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•16 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•16 years ago
|
Whiteboard: [(Av1 and) Bv1 is fixed1.9.1] → [fixed1.9.1b2: Av1]
Reporter | ||
Updated•15 years ago
|
Comment 24•6 years ago
|
||
triaging, moving bug to seamonkey not sure if this bug is still relevant
Product: Firefox → SeaMonkey
![]() |
||
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.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•