592774 - Re-enable Jetpack's memory debugging infrastructure when platform issues are resolved

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Atul Varma [:atul]]

Because the reliability of the entire memory debugging infrastructure of Jetpack is put in question by bug 592180, at the very least, I'm going to disable all of it by default and only re-enable it once that bug is fixed.

Right now this means I will be setting cfx's '--profile-memory' option to default to 'false' and disabling test-memory.testMemory (bug 590284).

Once bug 592180 is resolved, we'll re-enable these items and see where to go from there.

Comment 1

[Atul Varma [:atul]]

Attachment: patch

Comment 2

[Atul Varma [:atul]]

Comment on attachment 471190 [details] [diff] [review]
patch

Holy cow, our tests run a lot faster when memory profiling is disabled.

Comment 3

[Atul Varma [:atul]]

Pushed:

Bug 592774 - Disable Jetpack's memory debugging infrastructure until platform issues are resolved
Atul Varma [:atul]
http://hg.mozilla.org/labs/jetpack-sdk/rev/2dbc3c305f55

Comment 4

[Atul Varma [:atul]]

Er, I guess I'm going to re-open this bug as a reminder for us to apply a reverse-patch once the platform issues are resolved.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: reverse patch

Here's a patch that reverses the change, now that bug 592180 has been fixed.

Dietrich: alternately, we can reenable the test but leave --profile-memory false by default for speed (as I recall, testing sped up significantly when we disabled it).

Comment 7

[Dietrich Ayala (:dietrich)]

yeah, performance (hurts when you're trying to rapidly debug something, eg doing cfx run 300x a day. also, IIRC the output obscured other things, require having to scroll the terminal, which was annoying.

Comment 8

[Dietrich Ayala (:dietrich)]

bug 592180 isn't marked fixed, no comments since last august.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Attachment: reverse patch v2

(In reply to comment #7)
> yeah, performance (hurts when you're trying to rapidly debug something, eg
> doing cfx run 300x a day. also, IIRC the output obscured other things, require
> having to scroll the terminal, which was annoying.

Ok, let's reenable the test but leave memory profiling disabled by default.  Here's a patch that does that.


(In reply to comment #8)
> bug 592180 isn't marked fixed, no comments since last august.

Erm, right, I meant bug 590284.

Comment 10

[Myk Melez [:myk] [@mykmelez]]

https://github.com/mozilla/addon-sdk/commit/8b75b9a7f76410819d9abfec1f383480110ce99f

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Reopening.  Apparently those platform issues haven't been completely resolved, as the fix broke tests on some platforms.

https://github.com/mozilla/addon-sdk/commit/30e67fab5fefc4129188a1864555ca95433719ff

http://tinderbox.mozilla.org/showlog.cgi?log=Jetpack/1303512147.1303512272.15189.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=Jetpack/1303505544.1303506072.23425.gz&fulltext=1

In #jsapi:

(04:03:23 PM) bhackett: myk: that test doesn't look like it's accounting for the conservative GC which spidermonkey uses

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Furthermore:

(04:03:49 PM) bhackett: even if all references to an object from the JS stack and JS heap have been removed, there can be references on the C stack which the GC uses to keep the object alive

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(automatic reprioritization of 1.0 bugs)

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(Pushing all open bugs to the --- milestone for the new triage system)

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Unassigning myself from bugs I am not actively working on.

Comment 16

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562