Closed
Bug 1085727
Opened 10 years ago
Closed 10 years ago
Some DMD tweaks
Categories
(Core :: DMD, defect)
Core
DMD
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(3 files)
4.20 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
25.49 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
11.00 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
Three DMD tweaks coming up.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8508357 -
Flags: review?(continuation)
Comment 2•10 years ago
|
||
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+
Assignee | ||
Comment 3•10 years ago
|
||
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)
Assignee | ||
Comment 4•10 years ago
|
||
This is quite useful. I will characterise the use of basename as a "pragmatic hack".
Attachment #8508363 -
Flags: review?(continuation)
Comment 5•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #8508363 -
Flags: review?(continuation) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(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.
Assignee | ||
Comment 7•10 years ago
|
||
Thank you for the fast reviews.
Comment 8•10 years ago
|
||
(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.
Assignee | ||
Comment 9•10 years ago
|
||
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•10 years ago
|
||
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
Assignee | ||
Comment 11•10 years ago
|
||
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•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3e82193dab58 https://hg.mozilla.org/mozilla-central/rev/e6550843ba9f https://hg.mozilla.org/mozilla-central/rev/04015991eea0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•9 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•