If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

DMD: Add memory reporters for its own data

RESOLVED FIXED in Firefox 19

Status

()

Core
DMD
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: njn)

Tracking

Trunk
mozilla20
Points:
---

Firefox Tracking Flags

(firefox19 fixed, firefox20 fixed, b2g18 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Right now all memory used by newdmd shows up as heap-unclassified.  This is problematic because if you're running newdmd and you want to know how much heap-unclassified you actually have, you must run a full DMD dump and manually subtract DMD's memory usage out from heap-unclassified.

newdmd doesn't use memory reporters because it can't call into libxul.  But libxul can call into newdmd, so all we need to do is register a memory reporter in nsMemoryReporterManager.cpp (say) which calls into newdmd.
(Assignee)

Comment 1

5 years ago
I've mostly completed this.  On Linux64 with techcrunch.com open I have this:

├───73,210,576 B (38.50%) -- dmd
│   ├──41,943,120 B (22.06%) ── live-block-table
│   ├──27,073,072 B (14.24%) ── stack-traces
│   └───4,194,384 B (02.21%) ── stack-trace-table

At start-up it was 35 MiB.  On 32-bits these will all be halved, but yeah, some improvements are needed.
(Assignee)

Comment 2

5 years ago
With --sample-below=4093, for the techcrunch.com case I get this:

├────9,473,392 B (06.28%) -- dmd
│    ├──4,733,664 B (03.14%) ── stack-traces
│    ├──2,621,520 B (01.74%) ── live-block-table
│    ├──2,097,232 B (01.39%) ── stack-trace-table

Quite the difference.
(Assignee)

Updated

5 years ago
Summary: Add proper memory reporters for newdmd's memory → DMD: Add memory reporters for its own data
(Assignee)

Comment 3

5 years ago
Created attachment 691061 [details] [diff] [review]
Add a memory reporter for DMD's data.

This patch:

- Adds a memory reporter for DMD. 
  
- Prints more stats at the end of each Dump() call.  
  
- Reduces the initial sizes of some tables, making them suitable for a default
  sample-below value of 4093.
Attachment #691061 - Flags: review?(justin.lebar+bug)
(Assignee)

Updated

5 years ago
Assignee: nobody → n.nethercote
(Assignee)

Comment 4

5 years ago
Created attachment 691107 [details] [diff] [review]
Add "explicit/dmd/other" report.  WONTUSE.

This patch adds Sizes::mOther, for everything else allocated by DMD.  It's
implemented by measuring all allocations done in InfallibleAllocPolicy and
subtracting the other measurements from that number.

Unfortunately, this requires a global counter, which needs protection with a
mutex.  I didn't want to create a new mutex that's called on every
malloc/free (sounded expensive), so I took advantage of the existing
gStateLock.  I then had to rejig things a bit so that all of DMD's allocations
took place in places where we already held gStateLock, which was a bit
awkward.  Thread::Fetch() was the main casualty.

After all this, the amount of "other" with techcrunch.com open was a measly
416 bytes.  This is the thread-local storage, the copy of the $DMD env var,
and gStateLock.

These non-trivial modifications aren't worth it for the tiny improvement in
coverage.  But, having written the patch, I'm posting it here because it might
be a useful reference at some point.
(Reporter)

Comment 5

5 years ago
Yay.

>+  // Stats are non-deterministic, so don't show them in test mode.
>+  if (gMode != Test) {
>+    Sizes sizes;
>+    SizeOf(&sizes);
>+
>+    WriteTitle("Execution measurements\n");
>+
>+    W("Stack traces:        %10s bytes\n",
>+      Show(sizes.mStackTraces, gBuf1, kBufLen));
>+
>+    W("Stack trace table:   %10s bytes (%s entries, %s used)\n",
>+      Show(sizes.mStackTraceTable,       gBuf1, kBufLen),
>+      Show(gStackTraceTable->capacity(), gBuf2, kBufLen),
>+      Show(gStackTraceTable->count(),    gBuf3, kBufLen));
>+
>+    W("Live block table:    %10s bytes (%s entries, %s used)\n",
>+      Show(sizes.mLiveBlockTable,       gBuf1, kBufLen),
>+      Show(gLiveBlockTable->capacity(), gBuf2, kBufLen),
>+      Show(gLiveBlockTable->count(),    gBuf3, kBufLen));
>+
>+    W("Double-report table: %10s bytes (%s entries, %s used)\n",
>+      Show(sizes.mDoubleReportTable,                 gBuf1, kBufLen),
>+      Show(gDoubleReportBlockGroupTable->capacity(), gBuf2, kBufLen),
>+      Show(gDoubleReportBlockGroupTable->count(),    gBuf3, kBufLen));
>+
>+    size_t unreportedSize =
>+      unreportedBlockGroupTable.sizeOfIncludingThis(MallocSizeOf);
>+    W("Unreported table:    %10s bytes (%s entries, %s used)\n",
>+      Show(unreportedSize,                       gBuf1, kBufLen),
>+      Show(unreportedBlockGroupTable.capacity(), gBuf2, kBufLen),
>+      Show(unreportedBlockGroupTable.count(),    gBuf3, kBufLen));
>+
>+    size_t reportedSize =
>+      reportedBlockGroupTable.sizeOfIncludingThis(MallocSizeOf);
>+    W("Reported table:      %10s bytes (%s entries, %s used)\n",
>+      Show(reportedSize,                       gBuf1, kBufLen),
>+      Show(reportedBlockGroupTable.capacity(), gBuf2, kBufLen),
>+      Show(reportedBlockGroupTable.count(),    gBuf3, kBufLen));
>+
>+    W("\n");
>+  }

Is it worth indicating which of these data structures are persistent and which
are transient?
(Reporter)

Updated

5 years ago
Attachment #691061 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 6

5 years ago
> Is it worth indicating which of these data structures are persistent and
> which
> are transient?

In comments or in output?
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cbe7a6b5395


> In comments or in output?

I ended up putting it in the output.  Example:

  Data structures that persist after Dump() ends:
    Stack traces:         1,932,528 bytes
    Stack trace table:      262,224 bytes (16,384 entries, 9,291 used)
    Live block table:     1,310,800 bytes (16,384 entries, 6,426 used)

  Data structures that are cleared after Dump() ends:
    Double-report table:        400 bytes (4 entries, 0 used)
    Unreported table:       163,840 bytes (2,048 entries, 1,508 used)
    Reported table:         327,680 bytes (4,096 entries, 1,551 used)
(Reporter)

Comment 8

5 years ago
> I ended up putting it in the output.

Perfect, thanks.

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/3cbe7a6b5395
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(Reporter)

Comment 10

5 years ago
Comment on attachment 691061 [details] [diff] [review]
Add a memory reporter for DMD's data.

[Triage Comment]
DMD is npotb.
Attachment #691061 - Flags: approval-mozilla-b2g18+
Attachment #691061 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/b7b17631f55d
https://hg.mozilla.org/releases/mozilla-b2g18/rev/9f2804c1a2d8
status-b2g18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
(Assignee)

Updated

5 years ago
Component: General → DMD
You need to log in before you can comment on or make changes to this bug.