1371038 - Add properly packaged opt nightly DMD builds

Status: RESOLVED INCOMPLETE

(Release Engineering :: General, enhancement, P3)

Description

[Eric Rahm [:erahm]]

It would help users to debug high heap-unclassified issues if I could point them at a properly packaged opt DMD build (debug is a bit too slow with DMD enabled to be used for long periods of time).

Basically my use case is:
#1 - User files memshrink bug with high heap-unclassified
#2 - I want them to be able to download and install a nightly with DMD enabled
#3 - They run for a day, generate a DMD report [1], run 'dmd.py' on the output [2] and hand that back to us

So this means the build should have 'libdmd.so' (or dylib or dll) packaged with it along with dmd.py and it's associated scripts. Unfortunately we need to do the 'dmd.py' run on the users machine so that we have proper symbols.

Steps to verify things are working properly (on Linux for simplicity):

#1) mozdownload/mozinstall a nightly DMD build from automation (or do it manually)
#2) launch with: |DMD=1 LD_PRELOAD=$INSTALL_BIN_DIR/libdmd.so LD_LIBRARY_PATH=$INSTALL_BIN_DIR $INSTALL_BIN_DIR/firefox|. Note with mach this is a bit simpler: |mach run --dmd|)
#3) Go to about:memory, click 'Save' under 'Save DMD output'
#4) Run generated json.gz files through dmd.py (file names are output to console), ie:

> $INSTALL_BIN_DIR/dmd.py /tmp/dmd-1496869555-25274.json.gz

Verify that stacks are symbolicated.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/DMD#Trigger_7
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/DMD#Processing_the_output_2

Comment 1

[Nick Alexander :nalexander [he/him]]

There are two things here.

1. Package {Nightly, --enable-dmd, --disable-debug} builds for testers.  This has the side-benefit of enabling artifact builds with --enable-dmd, which should make it easier for frontend-devs to provide DMD reports.

I see a lot of --enable-dmd builds, but they are all --enable-debug.  (I think.)  erahm suggests those aren't useful for testers (since they're too slow for long term use).  Is there a reason we're building them?  Could we just turn those into --disable-debug builds, or do we want the debug information for something in automation?

If we can't turn those debug builds into opt builds, let's make this ticket track adding the equivalent opt-builds.  This ticket should be triaged, but if we don't get a response we can try to get coop (or somebody else from releng) to get this picked up.

2. Package dmd.py, etc, into the package.  This is trickier, and makes a number of assumptions about the user's runtime environment.  Would it be feasible to turn dmd.py into a JS addon (like the stuff that supports the Gecko profiler) or somehow make this require less of the user?  We can definitely put dmd.py someplace, but what is documented is always going to be hard for non-Mozilla contributors.

Comment 2

[Eric Rahm [:erahm]]

(In reply to Nick Alexander :nalexander from comment #1)
> There are two things here.
> 
> 1. Package {Nightly, --enable-dmd, --disable-debug} builds for testers. 
> This has the side-benefit of enabling artifact builds with --enable-dmd,
> which should make it easier for frontend-devs to provide DMD reports.
> 
> I see a lot of --enable-dmd builds, but they are all --enable-debug.  (I
> think.)  erahm suggests those aren't useful for testers (since they're too
> slow for long term use).  Is there a reason we're building them?  Could we
> just turn those into --disable-debug builds, or do we want the debug
> information for something in automation?

I think we just wanted something that builds DMD to make sure it doesn't get inadvertently broken. We need to be able to symbolicate stacks, dmd.py does that locally with addr2line (so we expect whatever addr2line/atos/bpsyms requires to be present). If we had some sort of service that I could throw stacks at and it spit out symbolicated versions of them I'd be happy. I feel like we must have that for soccorro and telemetry, but maybe someone can elaborate.

> If we can't turn those debug builds into opt builds, let's make this ticket
> track adding the equivalent opt-builds.  This ticket should be triaged, but
> if we don't get a response we can try to get coop (or somebody else from
> releng) to get this picked up.

I *think* this would be fine, we'd just make sure we have a way to translate the stacks.

> 2. Package dmd.py, etc, into the package.  This is trickier, and makes a
> number of assumptions about the user's runtime environment.  Would it be
> feasible to turn dmd.py into a JS addon (like the stuff that supports the
> Gecko profiler) or somehow make this require less of the user?  We can
> definitely put dmd.py someplace, but what is documented is always going to
> be hard for non-Mozilla contributors.

I imagine that would be a bit involved to do an addon (aren't those going away?), I agree our workflow is not geared towards non-contributors :(

Comment 3

[Nicholas Nethercote [inactive]]

> > I see a lot of --enable-dmd builds, but they are all --enable-debug.
> 
> I think we just wanted something that builds DMD to make sure it doesn't get
> inadvertently broken.

That's correct, where "inadvertently broken" covers both the build and the extensive xpcshell test at memory/replace/dmd/test/test_dmd.js, which I definitely want to keeping running on Windows, Mac and Linux, because DMD involves stack traces, which can easily become broken on one platform but not others.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Nicholas Nethercote [:njn] from comment #3)
> > > I see a lot of --enable-dmd builds, but they are all --enable-debug.
> > 
> > I think we just wanted something that builds DMD to make sure it doesn't get
> > inadvertently broken.
> 
> That's correct, where "inadvertently broken" covers both the build and the
> extensive xpcshell test at memory/replace/dmd/test/test_dmd.js, which I
> definitely want to keeping running on Windows, Mac and Linux, because DMD
> involves stack traces, which can easily become broken on one platform but
> not others.

njn: I assume these stack traces rely on --enable-debug, so it is *not* suitable to change the automation builds to be --disable-debug?

Comment 5

[Eric Rahm [:erahm]]

I have an opt (non --enable-debug) build and can symbolicate fine using $OBJDIR/dist/bin/dmd.py, I just don't know if that works for packaged builds or not. I'll see if I can get the test to run.

Comment 6

[Eric Rahm [:erahm]]

Yeah |./mach test memory/replace/dmd/test/| works fine, but again I'm not sure what automation does.

Comment 7

[Nicholas Nethercote [inactive]]

As erahm say, I *think* opt builds should work equally well. The choice to enable DMD for debug builds was somewhat arbitrary; I didn't want to enable it for all builds because it's not a shipped component, and debug-only seemed like a reasonable choice that gave multi-platform coverage.

Comment 8

[(not currently active) Ted Mielczarek]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #2)
> I think we just wanted something that builds DMD to make sure it doesn't get
> inadvertently broken. We need to be able to symbolicate stacks, dmd.py does
> that locally with addr2line (so we expect whatever addr2line/atos/bpsyms
> requires to be present). If we had some sort of service that I could throw
> stacks at and it spit out symbolicated versions of them I'd be happy. I feel
> like we must have that for soccorro and telemetry, but maybe someone can
> elaborate.

For builds that we ship (nightlies, releases) we upload symbols to crash-stats, and we have a server at symbolapi.mozilla.org which currently runs this code: https://github.com/vdjeric/Snappy-Symbolication-Server that the profiler uses to symbolicate stacks.

Additionally, nightly builds are shipped with function symbols, so you should technically have all the info you need locally for getting good stacks, it just might not be easy without something like addr2line. Do DMD stacks get printed with our internal NS_StackWalk machinery? I've never quite understood why that doesn't produce good results all the time.

Comment 9

[Nick Alexander :nalexander [he/him]]

I did a little testing. Notes:

- an artifact build cannot have --enable-dmd, since that implies --enable-jemalloc, which requires a compile environment.  That's understandable, but it does mean that any checks (even in tooling) that look at the build configuration to figure out if DMD is enabled will fail for artifact builds, no matter if the actual artifacts being used have support for DMD enabled.

- the test in |mach run| doesn't check the build configuration, it looks for libdmd.so (or equivalent).  In addition, all debug builds in automation have --enable-dmd.

*That means that --enable-debug --enable-artifact-builds currently have DMD support right now!*

- I tested on Mac OS X and found we need to add libdmd.dylib around http://searchfox.org/mozilla-central/rev/a798ee4fc323f9387b7576dbed177859d29d09b7/python/mozbuild/mozbuild/artifacts.py#352, but then everything should work on Mac.  However, it's so slow as to be unuseable -- perhaps just capturing these stack traces is too slow on Mac in general?  We should try with an opt build.

- I tested on Linux and found that DMD worked just fine... but the artifact build doesn't put dmd.py into the correct place, so I couldn't process with dmd.py in the source directory.  I think we can probably add a |mach dmd ...| that "just works" from the source directory, addressing the Python module import errors that probably would have been fixed if dmd.py was in $OBJDIR/dist/bin, as is expected.  (It's not since the artifact build can't have --enable-dmd; see above.)

All that suggests that we just need some new --enable-dmd opt builds and we'll be close to artifact build success!

Comment 10

[Nick Alexander :nalexander [he/him]]

catlee: I see you're triage owner.  Who is the right person to add these build types and start scheduling them (perhaps just once a day)?  Presumably this won't bump existing work, but perhaps you can estimate availability?

Comment 11

[Chris AtLee [:catlee]]

This is relatively painful to do in buildbot, and relatively simple to do in taskcluster. I'd like to wait a bit longer to see if cross-compiled builds work out.

We can revisit in July.

Comment 12

[Eric Rahm [:erahm]]

(In reply to Chris AtLee [:catlee] from comment #11)
> This is relatively painful to do in buildbot, and relatively simple to do in
> taskcluster. I'd like to wait a bit longer to see if cross-compiled builds
> work out.
> 
> We can revisit in July.

You've stated elsewhere that you're targeting September for a go/no-go on cross-compiled builds. Regardless, can we get something working on Linux and Windows and defer packaging mac to whenever we decide cross-compiled builds are working?

Comment 13

[Chris AtLee [:catlee]]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #12)
> (In reply to Chris AtLee [:catlee] from comment #11)
> > This is relatively painful to do in buildbot, and relatively simple to do in
> > taskcluster. I'd like to wait a bit longer to see if cross-compiled builds
> > work out.
> > 
> > We can revisit in July.
> 
> You've stated elsewhere that you're targeting September for a go/no-go on
> cross-compiled builds. Regardless, can we get something working on Linux and
> Windows and defer packaging mac to whenever we decide cross-compiled builds
> are working?

Yes, absolutely. I assumed this bug was OSX specific from skimming comment #0.

Comment 14

[Chris AtLee [:catlee]]

Does these builds need to use exactly the same toolchain as the nightly builds?

If not, then standing these up as extra nightly builds for Windows in taskcluster should be relatively easy.

Linux32/64 are already completely defined in TC, so there are no blockers for those platforms.

Comment 15

[Nick Alexander :nalexander [he/him]]

(In reply to Chris AtLee [:catlee] from comment #14)
> Does these builds need to use exactly the same toolchain as the nightly
> builds?

No, I don't think so, but erahm or njn will have the final say.

> If not, then standing these up as extra nightly builds for Windows in
> taskcluster should be relatively easy.
> 
> Linux32/64 are already completely defined in TC, so there are no blockers
> for those platforms.

Windows and Linux in TC would be a big step forward; if we could do the easy 80% quickly that would be awesome!  (DMD in general might be too slow to use in Mac OS X; I don't really know.  Certainly it's too slow with --enable-debug!)

Comment 16

[Chris AtLee [:catlee]]

I have builds working in TC for win32,win64,linux64 and macos:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e1efde06c4ea0e09ae67adaf50a596bb1adfdd6

These don't package up dmd.py, or any other required scripts.

Comment 17

[Eric Rahm [:erahm]]

(In reply to Chris AtLee [:catlee] from comment #16)
> I have builds working in TC for win32,win64,linux64 and macos:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=2e1efde06c4ea0e09ae67adaf50a596bb1adfdd6

I confirmed the Linux64 build works and can generate DMD output. I copied over my local dmd scripts and was able to produce symbolicated output so that's pretty awesome!

> These don't package up dmd.py, or any other required scripts.

The required scripts are listed here [1], locally they end up in $OBJDIR/dist/bin for me. They're not present in the packaged build for the try job though.

[1] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/build/moz.build#28-36

Comment 18

[Eric Rahm [:erahm]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)

> For builds that we ship (nightlies, releases) we upload symbols to
> crash-stats, and we have a server at symbolapi.mozilla.org which currently
> runs this code: https://github.com/vdjeric/Snappy-Symbolication-Server that
> the profiler uses to symbolicate stacks.

Is there a slightly more robust example of using this? It will definitely be helpful for parsing reports from less technical users where I might not know which build they're using.

> Additionally, nightly builds are shipped with function symbols, so you
> should technically have all the info you need locally for getting good
> stacks, it just might not be easy without something like addr2line.

Yeah I can symbolicate as long as I have the full build downloaded.

> Do DMD
> stacks get printed with our internal NS_StackWalk machinery? I've never
> quite understood why that doesn't produce good results all the time.

Our DMD JSON output looks something like this:

>  "NaQ": "#00: ???[/home/erahm/dev/tmp/firefox/libxul.so +0x2ab908a]",
>  "KcP": "#00: ???[/home/erahm/dev/tmp/firefox/libxul.so +0x40f919a]",
>  "MHL": "#00: ???[/home/erahm/dev/tmp/firefox/libxul.so +0xa86bee]",
>  "LfN": "#00: ???[/home/erahm/dev/tmp/firefox/libxul.so +0xa5670f]",
>  "MYa": "#00: ???[/home/erahm/dev/tmp/firefox/libxul.so +0x175bbd1]",

That output essentially comes from MozDescribeCodeAddress [1]. Underneath the hood that's using dladdr, I believe that will only get symbols for exported functions on Linux hence the janky output. OTOH I think the mac implementation includes exported symbols so we get more useful output (albeit much slower).

Our dmd.py script then blindly pumps those through the |fixSymbols| function [2] from fix_stack_*.py

[1] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/xpcom/base/CodeAddressService.h#148
[2] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/memory/replace/dmd/dmd.py#207-243

Comment 19

[Chris AtLee [:catlee]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b198a92c20ee936db303929d1f70885f48bd706

should package up dmd.py for linux and windows. broken for OSX, not sure why yet.

Comment 20

[Chris AtLee [:catlee]]

Attachment: Bug 1371038: Clean up old aurora cron entries

Review commit: https://reviewboard.mozilla.org/r/151812/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151812/

Comment 21

[Chris AtLee [:catlee]]

Attachment: Bug 1371038: Remove unused stylo tasks target

Review commit: https://reviewboard.mozilla.org/r/151814/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151814/

Comment 22

[Chris AtLee [:catlee]]

Attachment: Bug 1371038: Add configs for doing DMD builds

This adds the mozconfigs, mozharness configs and taskcluster changes required
to create optimized DMD builds for linux64, win32, win64 and macosx64.

These builds will happen nightly on mozilla-central

We also add support for custom build variants on Windows (or other generic
worker environments).

Review commit: https://reviewboard.mozilla.org/r/151816/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151816/

Comment 23

[Justin Wood (:Callek)]

Comment on attachment 8880464 [details]
Bug 1371038: Clean up old aurora cron entries

https://reviewboard.mozilla.org/r/151812/#review156824

Comment 24

[Justin Wood (:Callek)]

Comment on attachment 8880465 [details]
Bug 1371038: Remove unused stylo tasks target

https://reviewboard.mozilla.org/r/151814/#review156826

Comment 25

[(not currently active) Ted Mielczarek]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #18)
> (In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)
> 
> > For builds that we ship (nightlies, releases) we upload symbols to
> > crash-stats, and we have a server at symbolapi.mozilla.org which currently
> > runs this code: https://github.com/vdjeric/Snappy-Symbolication-Server that
> > the profiler uses to symbolicate stacks.
> 
> Is there a slightly more robust example of using this? It will definitely be
> helpful for parsing reports from less technical users where I might not know
> which build they're using.

That service is in the process of being replaced. The new code is here:
https://github.com/mozilla-services/tecken/

That repo has a link to docs which should be useful. It does require you to know the Breakpad debug ID for each module, so it can locate the right symbols. We have an API in Firefox now to get the list of loaded modules, so maybe you can tweak DMD to print the list of modules in its output with their debug IDs? Then you could have a script that converted that into a symbolapi request.

> That output essentially comes from MozDescribeCodeAddress [1]. Underneath
> the hood that's using dladdr, I believe that will only get symbols for
> exported functions on Linux hence the janky output. OTOH I think the mac
> implementation includes exported symbols so we get more useful output
> (albeit much slower).

We should rewrite this all in Rust and make it work better someday. :)

Comment 26

[Justin Wood (:Callek)]

Comment on attachment 8880466 [details]
Bug 1371038: Add configs for doing DMD builds

https://reviewboard.mozilla.org/r/151816/#review156830

::: taskcluster/ci/build/linux.yml:29
(Diff revision 1)
>  
> +linux64-dmd/opt:
> +    description: "Linux64 DMD Opt"
> +    index:
> +        product: firefox
> +        job-name: linux64-dmd-opt

I do wonder if we need to call out that this is a nightly. (that affects index and other things)

But then we likely would have more taskgraph work to not send these off for signing, beetmover, etc.

Comment 27

[Chris AtLee [:catlee]]

Comment on attachment 8880466 [details]
Bug 1371038: Add configs for doing DMD builds

https://reviewboard.mozilla.org/r/151816/#review156830

> I do wonder if we need to call out that this is a nightly. (that affects index and other things)
> 
> But then we likely would have more taskgraph work to not send these off for signing, beetmover, etc.

My thinking was that these are regular builds, that just happen to run on a nightly schedule.

I don't think we're planning on shipping these in any official way, and I don't think they need signing, updates, etc. either.

Comment 28

[Chris AtLee [:catlee]]

Eric, are my assumptions in comment 27 correct?

Comment 29

[(not currently active) Ted Mielczarek]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #25)
> That service is in the process of being replaced. The new code is here:
> https://github.com/mozilla-services/tecken/

FYI, there are staging/dev instances of tecken running at symbols.stage.mozaws.net / symbols.dev.mozaws.net if you'd like to test the API. It should work better than the production symbolapi service.

Comment 30

[Eric Rahm [:erahm]]

(In reply to Chris AtLee [:catlee] from comment #28)
> Eric, are my assumptions in comment 27 correct?

I want to hand these out to non-moco users, so it seems like signing might be a good idea? For example if I try to run the osx build from your try push by clicking on the 'Nightly.app' icon in the dmg file I get a "Nightly.app" can't be opened because it is from an unidentified developer error. I'm not sure how this affects other platforms.

For more technical users we can have them use mozdownload/mozinstall/mozrunner and bypass this issue, but that's a bit of a stretch (particularly on Windows).

Additionally I think we want to have symbols available on the symbol server, it sounded like that was gating on this being a "real" nightly but maybe I'm wrong.

Comment 31

[Eric Rahm [:erahm]]

Comment on attachment 8880466 [details]
Bug 1371038: Add configs for doing DMD builds

https://reviewboard.mozilla.org/r/151816/#review156890

::: browser/installer/package-manifest.in:834
(Diff revision 1)
>  @RESPATH@/components/SanityTest.js
>  #endif
>  
> -#ifdef MOZ_MULET
> -#include ../../b2g/installer/package-manifest.in
> +#ifdef MOZ_DMD
> +; DMD
> +@RESPATH@/dmd.py

Unless I'm missing something I think we need to specify the [platform specific stack fixers](http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/build/moz.build#28-36) as well.

Comment 32

[Eric Rahm [:erahm]]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #30)
> (In reply to Chris AtLee [:catlee] from comment #28)
> > Eric, are my assumptions in comment 27 correct?
> 
> I want to hand these out to non-moco users, so it seems like signing might
> be a good idea? For example if I try to run the osx build from your try push
> by clicking on the 'Nightly.app' icon in the dmg file I get a "Nightly.app"
> can't be opened because it is from an unidentified developer error. I'm not
> sure how this affects other platforms.
> 
> For more technical users we can have them use
> mozdownload/mozinstall/mozrunner and bypass this issue, but that's a bit of
> a stretch (particularly on Windows).
> 
> Additionally I think we want to have symbols available on the symbol server,
> it sounded like that was gating on this being a "real" nightly but maybe I'm
> wrong.

Sorry I should have noted that those might be fine as follow ups, just having builds I can point users to is a big a improvement. So just to be clear:

#1 - Follow up to add symbols to our symbol server (that might be easy to do in this bug, I'm not sure the level of effort)
#2 - Follow up to hook in code signing

Comment 33

[Chris AtLee [:catlee]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae26d16a4717

Comment 34

[Chris AtLee [:catlee]]

Comment on attachment 8880464 [details]
Bug 1371038: Clean up old aurora cron entries

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151812/diff/1-2/

Comment 35

[Chris AtLee [:catlee]]

Comment on attachment 8880465 [details]
Bug 1371038: Remove unused stylo tasks target

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151814/diff/1-2/

Comment 36

[Chris AtLee [:catlee]]

Comment on attachment 8880466 [details]
Bug 1371038: Add configs for doing DMD builds

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151816/diff/1-2/

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcd685905323
Clean up old aurora cron entries r=Callek
https://hg.mozilla.org/integration/autoland/rev/e3de568a6fe4
Remove unused stylo tasks target r=Callek
https://hg.mozilla.org/integration/autoland/rev/a16d92beb70d
Add configs for doing DMD builds r=Callek

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bcd685905323
https://hg.mozilla.org/mozilla-central/rev/e3de568a6fe4
https://hg.mozilla.org/mozilla-central/rev/a16d92beb70d

Comment 39

[Chris AtLee [:catlee]]

Still need to set up the hooks, and then do signing & symbols.

Comment 40

[Chris AtLee [:catlee]]

quick note to say that these have been running nightly and are available via the taskcluster index

e.g.

https://tools.taskcluster.net/index/artifacts/gecko.v2.mozilla-central.latest.firefox.win64-dmd-opt

https://index.taskcluster.net/v1/task/gecko.v2.mozilla-central.latest.firefox.win64-dmd-opt/artifacts/public/build/install/sea/target.installer.exe

Comment 41

[Mike Hommey [:glandium]]

Why are we not simply making dmd the default for milestone.is_nightly?

Comment 42

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #41)
> Why are we not simply making dmd the default for milestone.is_nightly?

Performance concerns?

Comment 43

[Mike Hommey [:glandium]]

(In reply to Nathan Froyd [:froydnj] from comment #42)
> Performance concerns?

DMD has no overhead if it's not enabled, except in mozStorageService.

Comment 44

[Mike Hommey [:glandium]]

*enabled at runtime

Comment 45

[Nathan Froyd [:froydnj]]

If there is zero performance impact, then we should just do it, it would be great to have instructions ready at hand for nightly users who experience weird heap-unclassified issues: here, restart your browser like so, use it as normal, and then when you notice problems, hit these buttons and send us output.

Comment 46

[Nicholas Nethercote [inactive]]

> If there is zero performance impact, then we should just do it

There's a codesize overhead, but it's probably pretty small. The other argument is that it moves us further away from what we ship on Release. But neither of these concerns are major.

Comment 47

[(not currently active) Ted Mielczarek]

Is there a reason we couldn't have it enabled for release builds, even? If it's just a small codesize overhead that seems fine given the utility it provides.

Comment 48

[Nicholas Nethercote [inactive]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #47)
> Is there a reason we couldn't have it enabled for release builds, even? If
> it's just a small codesize overhead that seems fine given the utility it
> provides.

That would certainly eliminate the "what we test doesn't match what we ship" objection :)

My concerns:

- To enable DMD at runtime you have to set multiple environment variables, and they differ on each platform. On a local build `./mach run --dmd` does it for you... does `mach` work in release builds? If not, we might need another script to hide the details.`

- When DMD is present in the build but not enabled at runtime (which would be the common case), about:memory has a "Save DMD output" section with a "Save" button within it, and that button is greyed out, which is potentially confusing. But about:memory is obscure enough that this probably doesn't matter. Perhaps we could add a link to the DMD MDN page.

- The post-processing steps are a little hairy for the average user, and we'd need to make sure that stacks get symbolized properly.

Basically, these are polish concerns that arise when you go from "this is a build-time flag and so only Firefox devs are likely to run it" to "anybody could potentially run this".

Comment 49

[Mike Hommey [:glandium]]

The multiple environment variables differing on each platform thing is planned to go away. So that's one less concern.

OTOH, there are security concerns leading to replace-malloc not being enabled on releases, and replace-malloc is a prerequisite for DMD, so there's that.

Comment 50

[Chris AtLee [:catlee]]

What direction do we want to take here?

Comment 51

[Eric Rahm [:erahm]]

(In reply to Chris AtLee [:catlee] from comment #50)
> What direction do we want to take here?

I'm fine with enabling in release for Nightly, but I think we'd need to get glandium's sign-off per comment 49.

Comment 52

[Mike Hommey [:glandium]]

For the purpose of this bug, we can enable dmd on opt nightlies (but not on beta/release). And remove the extra dmd jobs, since they would be redundant. Making things easier to enable dmd can be done separately, the extra dmd jobs didn't solve that anyways.

Comment 53

[Chris AtLee [:catlee]]

If we're enabling dmd on opt (pgo) nightlies, should we also enable dmd on opt and pgo builds? We should try and keep nightly build configuration as close as possible to the CI build configuration.

Comment 54

[Mike Hommey [:glandium]]

No, cf. comment 49. Also, DMD doesn't really enable much that isn't already enabled on nightlies: replace-malloc is enabled on nightlies, and not on beta/release, so that's already an existing difference. And DMD doesn't add much on top of that, because most of DMD is simply a replace-malloc library.

Comment 55

[Nicholas Nethercote [inactive]]

(In reply to Mike Hommey [:glandium] from comment #54)
> No, cf. comment 49. Also, DMD doesn't really enable much that isn't already
> enabled on nightlies: replace-malloc is enabled on nightlies, and not on
> beta/release, so that's already an existing difference. And DMD doesn't add
> much on top of that, because most of DMD is simply a replace-malloc library.

Did you misread catlee's question? I think you're talking about beta/release, but he's talking about non-Nightly CI builds.

Comment 56

[Mike Hommey [:glandium]]

I thought he was talking about beta/release, because we don't have configuration differences between non-nightly CI for m-c and nightlies.

Comment 57

[Chris AtLee [:catlee]]

Given that the recent discussion on the bug have been focused on nightlies, I wanted to clarify whether enabling DMD for non-nightly CI builds was desired or not.

Sounds like we can enable in the 'nightly' mozconfig, which is used by all CI and nightly builds except for beta, release and esr branches.

Comment 58

[(not currently active) Ted Mielczarek]

Can we just make DMD enabled by default on builds that are not beta?

Comment 59

[Chris AtLee [:catlee]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #58)
> Can we just make DMD enabled by default on builds that are not beta?

I'm working on that in bug 1409739. I've addressed the build failures, but not sure how to handle the xpcshell test failure.

e.g. https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ab7f503759bcd0a565e21d0010416290697baab&selectedJob=138294316

Comment 60

[Eric Rahm [:erahm]]

It looks like finally landing bug 1409739 is causing 'cron(Ndmd)' failures due to:

> KeyError: 'nightly_dmd'

That appears to be coming from '.cron.yml' [1] which we modified in this bug. Chris do you have an idea of what's going on?

[1] https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/.cron.yml#81-91

Comment 61

[Eric Rahm [:erahm]]

(In reply to Eric Rahm [:erahm] from comment #60)
> It looks like finally landing bug 1409739 is causing 'cron(Ndmd)' failures
> due to:
> 
> > KeyError: 'nightly_dmd'
> 
> That appears to be coming from '.cron.yml' [1] which we modified in this
> bug. Chris do you have an idea of what's going on?
> 
> [1]
> https://searchfox.org/mozilla-central/rev/
> 80ac71c1c54af788b32e851192dfd2de2ec18e18/.cron.yml#81-91

Bug 1409739 comment 40 is the context here.

Comment 62

[Chris AtLee [:catlee]]

I removed support for the nightly_dmd target in https://hg.mozilla.org/mozilla-central/rev/74abe131db1b#l9.12

Should be straightforward to re-add that.

Comment 63

[Mike Hommey [:glandium]]

Why readd it? We don't need dmd nightlies.

Comment 64

[Julien Cristau [:jcristau]]

Closing for inactivity.