Closed Bug 1085727 Opened 10 years ago Closed 10 years ago

Some DMD tweaks

Categories

(Core :: DMD, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: n.nethercote, Assigned: n.nethercote)

Details

Attachments

(3 files)

Three DMD tweaks coming up.
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
Attachment #8508357 - Flags: review?(continuation) → review+
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.
Attachment #8508360 - Flags: review?(continuation)
This is quite useful. I will characterise the use of basename as a "pragmatic
hack".
Attachment #8508363 - Flags: review?(continuation)
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.
Attachment #8508360 - Flags: review?(continuation) → review+
Attachment #8508363 - Flags: review?(continuation) → review+
(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.
Thank you for the fast reviews.
(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.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.