819772 - DMD: Add memory reporters for its own data

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Justin Lebar (not reading bugmail)]

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.

Comment 1

[Nicholas Nethercote [inactive]]

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.

Comment 2

[Nicholas Nethercote [inactive]]

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.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 5

[Justin Lebar (not reading bugmail)]

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?

Comment 6

[Nicholas Nethercote [inactive]]

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

In comments or in output?

Comment 7

[Nicholas Nethercote [inactive]]

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)

Comment 8

[Justin Lebar (not reading bugmail)]

> I ended up putting it in the output.

Perfect, thanks.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3cbe7a6b5395

Comment 10

[Justin Lebar (not reading bugmail)]

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

[Triage Comment]
DMD is npotb.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b7b17631f55d
https://hg.mozilla.org/releases/mozilla-b2g18/rev/9f2804c1a2d8