1171647 - Minor refactoring of DMD to make it easier to add new modes

Status: RESOLVED FIXED

(Core :: DMD, defect)

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Define a new function to convert the mode to a string.

These are all kind of a matter of taste, but hopefully you agree with me.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4127a33255d9

The try run hasn't finished, but these changes are all pretty simple so hopefully it will be green.

Encapsulating the mode-string conversion into the options is much nicer in my opinion.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Make mode checking more generic in dmd.py.

On entry, we establish that the mode must be one of a few possibilities. The only case that ever matters is whether it is dark-matter or not. This change lets you add a new mode by modifying only a couple of lines of dmd.py. The extra checking keeps the strong invariant that we have only one of the known modes. The "if not" format is odd, but it didn't seem worth the effort to flip all of the branches.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: part 3 - Make the liveOrCumulative variables more generic.

There are a number of liveOrCumulative* variables and keys used, but that name isn't accurate any more if you add a third mode. Renaming to some kind of liveOrCumulativeOrWhatever format every time you add a new mode isn't going to scale well. Instead just give up on the non-specific specificity and just say, hey, these are heapRecords or heapBlocks or whatever.

Comment 4

[Andrew McCreight [:mccr8]]

See the patch in bug 1058178 for an example of how few changes it requires with these patches to add a new mode.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Remove redundant assertion for dark matter mode.

This is an unrelated thing I noticed. It seems silly to check for a condition and then immediately assert against it.

Comment 6

[Nicholas Nethercote [inactive]]

Comment on attachment 8615562 [details] [diff] [review]
part 2 - Make mode checking more generic in dmd.py.

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

As discussed on IRC, we can avoid this and instead just change the mode from "contents" to "live" at the very start of dmd.py.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c0b129ab295
https://hg.mozilla.org/integration/mozilla-inbound/rev/860fcc17b26b

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0c0b129ab295
https://hg.mozilla.org/mozilla-central/rev/860fcc17b26b