517475 - Stop running leak tests on debug builds?

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Chris AtLee [:catlee]]

Once we have unittests running on debug builds, is there any reason to continue running the current leak tests?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Nope.

Comment 2

[Chris AtLee [:catlee]]

Do the new tests need to be posting results to the graph server and/or be comparing leak/bloat logs?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Hmmm.  Actually, nothing would be running the trace-malloc leak test anymore with this change.  So that actually is a pretty important thing we need to catch; otherwise we wouldn't catch regressions in leaks of non-logged objects.

We probably don't need the max heap numbers anymore; the Tp memset numbers are better.  (Though there might be value in tracking memory use after just bringing up a firefox window.)

Comment 4

[Chris AtLee [:catlee]]

So we can stop running:
python leaktest.py -- -register
python leaktest.py -- -CreateProfile default
python leaktest.py -- -P default
python leaktest.py -l bloat.log

but keep:

python leaktest.py -- --trace-malloc malloc.log --shutdown-leaks=sdleak.log

?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #4)
> So we can stop running:
> python leaktest.py -- -register
> python leaktest.py -- -CreateProfile default
> python leaktest.py -- -P default

These three are sort of the "set things up for the rest" part, so I think they should probably stay.

> python leaktest.py -l bloat.log

Yes, this can go.

> but keep:
> 
> python leaktest.py -- --trace-malloc malloc.log --shutdown-leaks=sdleak.log

Yes.


(But aren't there a bunch of other steps for comparing the logs?)

Comment 6

[Chris AtLee [:catlee]]

(In reply to comment #5)
> (In reply to comment #4)
> > So we can stop running:
> > python leaktest.py -- -register
> > python leaktest.py -- -CreateProfile default
> > python leaktest.py -- -P default
> 
> These three are sort of the "set things up for the rest" part, so I think they
> should probably stay.
> 
> > python leaktest.py -l bloat.log
> 
> Yes, this can go.
> 
> > but keep:
> > 
> > python leaktest.py -- --trace-malloc malloc.log --shutdown-leaks=sdleak.log
> 
> Yes.
> 
> 
> (But aren't there a bunch of other steps for comparing the logs?)

Yes, the steps for comparing the logs for everything except malloc.log and sdleak.log would go as well, as would the graph server posts.

Comment 7

[Chris Cooper [:coop] (he/him)]

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [tested] removing unwanted steps from leak test build steps

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 482289 [details] [diff] [review]
[tested] removing unwanted steps from leak test build steps

Ran this in staging successfully.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Which steps are you removing?  It looks to me like this is removing the nsTraceRefcnt-based leak test (which makes sense to me) and also removing graph server posts for the trace-malloc-based leak test (which doesn't make sense to me).

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

> Which steps are you removing? 

I took out all the bloat.log steps based on the comments in this bug - and also the graph server posts as per comment 6:

> Yes, the steps for comparing the logs for everything except malloc.log and
> sdleak.log would go as well, as would the graph server posts.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Perhaps I misunderstood comment 6?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So what are we posting to the graph server and what are we not posting to the graph server?  We should still be posting the trace-malloc test data to the graph server.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [tested] removing unwanted steps from leak test build steps

You're right, I was taking out both graphserver posts. 

So now I'm taking out only:

TinderboxPrint: <a title="refcnt_leaks"....

And leaving in the following:

TinderboxPrint: <a title="trace_malloc_maxheap"....
TinderboxPrint: <a title="trace_malloc_allocs"....
TinderboxPrint: <a title="trace_malloc_leaks"....

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 482369 [details] [diff] [review]
[tested] removing unwanted steps from leak test build steps

http://hg.mozilla.org/build/buildbotcustom/rev/dd5d5d953b0b