Closed
Bug 819772
Opened 12 years ago
Closed 12 years ago
DMD: Add memory reporters for its own data
Categories
(Core :: DMD, defect)
Core
DMD
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: justin.lebar+bug, Assigned: n.nethercote)
References
Details
Attachments
(2 files)
10.54 KB,
patch
|
justin.lebar+bug
:
review+
justin.lebar+bug
:
approval-mozilla-aurora+
justin.lebar+bug
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
29.05 KB,
patch
|
Details | Diff | Splinter Review |
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•12 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•12 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•12 years ago
|
Summary: Add proper memory reporters for newdmd's memory → DMD: Add memory reporters for its own data
Assignee | ||
Comment 3•12 years ago
|
||
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•12 years ago
|
Assignee: nobody → n.nethercote
Assignee | ||
Comment 4•12 years ago
|
||
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•12 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•12 years ago
|
Attachment #691061 -
Flags: review?(justin.lebar+bug) → review+
Assignee | ||
Comment 6•12 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•12 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•12 years ago
|
||
> I ended up putting it in the output.
Perfect, thanks.
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3cbe7a6b5395
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Reporter | ||
Comment 10•11 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+
Comment 11•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b7b17631f55d https://hg.mozilla.org/releases/mozilla-b2g18/rev/9f2804c1a2d8
Assignee | ||
Updated•11 years ago
|
Component: General → DMD
You need to log in
before you can comment on or make changes to this bug.
Description
•