1121830 - DMD: add "num" field to blocks in the output

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Nicholas Nethercote [inactive]]

The DMD output can have *many* duplicate block entries, especially in cumulative mode. (1,000,000+ for the more common ones in longer-running sessions.)

Adding a "n" field to the block output that allows these to be aggregated will allow the output file size to shrink dramatically.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: DMD: add "n" property to blocks in the output

This patch only uses the "n" property for dead blocks, because that's where the
greatest potential benefit lies, but it could be used for live blocks as well.

On one test case (a complex PDF file) running with --mode=cumulative
--sample-below=1 this patch had the following effects.

- Change in running speed was negligible.

- Compressed output file size dropped from 8.8 to 5.0 MB.

- Compressed output file size dropped from 297 to 50 MB.

- dmd.py runtime (without stack fixing) dropped from 30 to 8 seconds.

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 8550023 [details] [diff] [review]
DMD: add "n" property to blocks in the output

Review of attachment 8550023 [details] [diff] [review]:
-----------------------------------------------------------------

Nice improvement.

::: memory/replace/dmd/DMD.cpp
@@ +1067,5 @@
> +void MaybeAddToDeadBlockTable(const DeadBlock& aDb)
> +{
> +  if (gOptions->IsCumulativeMode() && aDb.AllocStackTrace()) {
> +    AutoLockState lock;
> +    if (DeadBlockTable::Ptr p = gDeadBlockTable->lookup(aDb)) {

It would be more efficient to use lookupForAdd() here.

::: memory/replace/dmd/DMD.h
@@ +150,5 @@
>  //   // - 1: The original format. Implemented in bug 1044709.
>  //   // - 2: Added the "mode" property under "invocation". Added in bug 1094552.
>  //   // - 3: The "dmdEnvVar" property under "invocation" can now be |null| if
>  //   //      the |DMD| environment variable is not defined. Done in bug 1100851.
> +//   // - 4: Added the "n" property in "blockList" object. Done in bug 1121830.

nit: these are inconsistent about "implemented in" vs "added in" vs "done in".

@@ +191,5 @@
> +//       // The number of heap blocks with exactly the above properties. This
> +//       // is mandatory if it is greater than one, but omitted otherwise.
> +//       // (Blocks with identical properties don't have to be aggregated via
> +//       // this property, but it can greatly reduce output file size.)
> +//       "n": 5

"n" seems overly terse. Why not "num" or "copies" or something?  I had no idea what this bug was about from its name. ;)

::: memory/replace/dmd/dmd.py
@@ +419,5 @@
>  
> +        record.numBlocks  += n
> +        record.reqSize    += n * reqSize
> +        record.slopSize   += n * slopSize
> +        record.usableSize += n * usableSize

You should implement vector multiplication for records!  Just kidding.

Comment 3

[Nicholas Nethercote [inactive]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f168b7c2a2e3

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bf791ce96e8

Comment 5

[Carsten Book [:Tomcat]]

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