1013652 - Add DMD button in about:memory for DMD builds

Status: RESOLVED FIXED

(Toolkit :: about:memory, defect)

Description

[Andrew McCreight [:mccr8]]

I don't know how possible it is, but it would be nice if when I had a DMD build, I could just open up about:memory, type in a file name into some kind of text box, and click a button to get a DMD dump.  (Also, somebody needs to take an axe to the description of how to do this on the DMD wiki page, because it is a horrible mish mash of broken instructions.)

Comment 1

[Andrew McCreight [:mccr8]]

Nick cleaned the directions up, so I withdraw my complaints. ;)

Comment 2

[Andrew McCreight [:mccr8]]

Well, a button would still be nice.  But at least a novice like me can easily understand how to trigger a DMD dump now.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Add a "DMD" button to about:memory when DMD is enabled

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 8425960 [details] [diff] [review]
Add a "DMD" button to about:memory when DMD is enabled

Review of attachment 8425960 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +341,5 @@
>    appendButton(row4, GCAndCCAllLogDesc,
>                 saveGCLogAndVerboseCCLog, "Save verbose", 'saveLogsVerbose');
>  
> +  // This only succeeds in --enable-dmd builds.
> +  if (typeof DMDReportAndDump == 'function') {

Note that this may break if you fix DMD builds to not cause failures in the test_interfaces, but it should be easy enough to change this.

@@ +345,5 @@
> +  if (typeof DMDReportAndDump == 'function') {
> +    let env = Components.classes["@mozilla.org/process/environment;1"]
> +                        .getService(Components.interfaces.nsIEnvironment);
> +
> +    // Gray the button out if DMD isn't enabled at start-up.

nice

@@ +347,5 @@
> +                        .getService(Components.interfaces.nsIEnvironment);
> +
> +    // Gray the button out if DMD isn't enabled at start-up.
> +    let dmd = env.get('DMD');
> +    let disabled = dmd == '' || dmd === '0';

Does that need to be === ''?  I don't know enough about JS to say.

@@ +436,5 @@
>  {
>    dumpGCLogAndCCLog(true);
>  }
>  
> +function doDMD()

nit: saveDMDLog() instead?

Comment 5

[Nicholas Nethercote [inactive]]

> > +  // This only succeeds in --enable-dmd builds.
> > +  if (typeof DMDReportAndDump == 'function') {
> 
> Note that this may break if you fix DMD builds to not cause failures in the
> test_interfaces, but it should be easy enough to change this.

I learnt about that test yesterday because it broke when I tried enabling DMD by default on TBPL debug builds. I won't worry about it for now, but if you know what magic is required to avoid this DMD function breaking that test I'd be interested to hear about it.

Comment 6

[Andrew McCreight [:mccr8]]

You are sticking it as a property on window, so it is just accessible to everything, including content.  This isn't a big deal when it is just DMD builds, but it should be fixed for debug builds or people will be sad.  You could do something like add it to Components.utils, and just have it only do something in DMD builds.

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf53e366410f

Comment 8

[Nicholas Nethercote [inactive]]

Ah, in nsJSEnvironment.cpp we have this code

> #ifdef NS_TRACE_MALLOC
>   if (nsContentUtils::IsCallerChrome()) {
>     // Attempt to initialize TraceMalloc functions
>     ::JS_DefineFunctions(cx, aGlobalObj, TraceMallocFunctions);
>   }
> #endif
> 
> #ifdef MOZ_DMD
>   // Attempt to initialize DMD functions
>   ::JS_DefineFunctions(cx, aGlobalObj, DMDFunctions);
> #endif

I guess wrapping the DMD initialization in IsCallerChrome() would fix it, then?

Comment 9

[Andrew McCreight [:mccr8]]

Yeah, I guess we only end up defining the function on chrome globals, or something.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cf53e366410f