1085727 - Some DMD tweaks

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Nicholas Nethercote [inactive]]

Three DMD tweaks coming up.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - DMD: make some very short functions one-liners

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 8508357 [details] [diff] [review]
(part 1) - DMD: make some very short functions one-liners

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

::: memory/replace/dmd/DMD.cpp
@@ +323,3 @@
>  
> +  void Lock()   { EnterCriticalSection(&mCS); }
> +  void Unlock() { LeaveCriticalSection(&mCS);

nit: looks like an extra new line in here

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove dmd.py's -b option and make its behaviour the default

dmd.py's -b option (a.k.a. --show-all-block-sizes) is really useful. For
example, in a record like this:

  426 blocks in heap block record 1 of 14,442
  27,918,336 bytes (27,918,336 requested / 0 slop)
  6.22% of the heap (6.22% cumulative)

I used to regularly fire up python to compute |27918336 / 426.0| in order to
see if all the blocks are the same size. But with -b I get this extra line:

  Individual block sizes: 65,536 x 426

In fact, it's so useful that I think it should always be on, and there's no
need to be able to disable it. I.e. the option can be removed.

This patch removes -b and makes that behaviour the default, with the following
tweaks:

- It now puts that line before the "of the heap" line, instead of after it.
  
- It doesn't show that line if the record contains a single block, because the
  list is redundant in that case.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Print dmd.py's invocation at the top of its output

This is quite useful. I will characterise the use of basename as a "pragmatic
hack".

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 8508360 [details] [diff] [review]
(part 2) - Remove dmd.py's -b option and make its behaviour the default

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

Neat.

My only comment would be to see how long the list can get in a typical file, and consider filing a followup to truncate the list if it is long.  When I've looked at the output of something like this before, you can end up with really huge lists.  Though maybe the consolidation of multiple blocks of the same size is enough to avoid that.

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to Andrew McCreight [:mccr8] from comment #5)
> Comment on attachment 8508360 [details] [diff] [review]
> (part 2) - Remove dmd.py's -b option and make its behaviour the default
> 
> Review of attachment 8508360 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Neat.
> 
> My only comment would be to see how long the list can get in a typical file,
> and consider filing a followup to truncate the list if it is long.

I considered it. I looked at output for some large runs and this was the biggest one I saw:

> Individual block sizes: 1,142,784 x 6; 1,007,616; 978,944; 925,696; 798,720; 4 42,368; 405,504 x 2; 401,408; 397,312 x 2; 393,216; 389,120 x 2; 385,024; 380,92 8 x 2; 376,832; 372,736 x 2; 368,640 x 2; 364,544 x 2; 360,448 x 2; 356,352 x 2; 352,256; 348,160 x 2; 344,064; 339,968 x 2; 335,872 x 2; 331,776 x 2; 327,680 x 2; 323,584 x 2; 319,488; 315,392 x 2; 311,296; 307,200; 303,104 x 2; 299,008 x 2; 294,912 x 2; 290,816 x 2; 286,720; 282,624 x 4; 278,528; 274,432 x 4; 270,336 x 2; 266,240 x 2; 262,144 x 4; 258,048 x 3; 253,952 x 3; 249,856 x 2; 245,760 x 5; 241,664 x 3; 237,568 x 4; 233,472 x 2; 229,376 x 4; 225,280; 221,184 x 4; 21 7,088 x 3; 212,992 x 6; 208,896 x 2; 204,800 x 5; 200,704 x 2; 196,608 x 5; 192, 512 x 4; 188,416 x 4; 184,320 x 4; 180,224 x 4; 176,128 x 4; 172,032 x 6; 167,93 6 x 4; 163,840 x 5; 159,744 x 4; 155,648 x 3; 151,552 x 4; 147,456 x 3; 143,360 x 4; 139,264 x 4; 135,168 x 4; 131,072 x 22; 126,976 x 5; 122,880 x 4; 118,784 x 5; 114,688 x 6; 110,592 x 5; 106,496 x 3; 102,400 x 4; 98,304 x 6; 94,208 x 16; 90,112 x 2; 86,016 x 24; 81,920 x 15; 77,824 x 6; 73,728 x 7; 69,632 x 18; 65,5 36 x 142; 61,440 x 20; 57,344 x 11; 53,248 x 15; 49,152 x 54; 45,056 x 56; 40,96 0 x 11; 36,864 x 63; 32,768 x 158; 28,672 x 28; 24,576 x 31; 20,480 x 71; 16,384 x 122; 12,288 x 120; 8,192 x 231; 4,096 x 827; ~4,093 x 7,494

That's from the record for nsStringBuffer::Alloc() in a run with --max-frames=1, which is the worst case. Fairly big, but still less than a page, largely because the granularity of jemalloc's buckets is relatively coarse. So I didn't bother with anything fancy.

Comment 7

[Nicholas Nethercote [inactive]]

Thank you for the fast reviews.

Comment 8

[Andrew McCreight [:mccr8]]

(In reply to Nicholas Nethercote [:njn] from comment #6)
> That's from the record for nsStringBuffer::Alloc() in a run with
> --max-frames=1, which is the worst case. Fairly big, but still less than a
> page, largely because the granularity of jemalloc's buckets is relatively
> coarse. So I didn't bother with anything fancy.

Yeah, that's not bad.  Plus there's so much that is combined due to the sampling.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/450c187cbc1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cfce41bed5c
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bb832d0c539

Comment 10

[Carsten Book [:Tomcat]]

sorry had to back this out in https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=de805196bbc4 since one of this changes caused perma failure on 10.8 Debug Tests like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3275566&repo=mozilla-inbound

Comment 11

[Nicholas Nethercote [inactive]]

Attempt 2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e82193dab58
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6550843ba9f
https://hg.mozilla.org/integration/mozilla-inbound/rev/04015991eea0

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3e82193dab58
https://hg.mozilla.org/mozilla-central/rev/e6550843ba9f
https://hg.mozilla.org/mozilla-central/rev/04015991eea0