Closed Bug 1056954 Opened 5 years ago Closed 5 years ago

Normalize 'pid' values when diffing

Categories

(Toolkit :: about:memory, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: erahm, Assigned: erahm)

References

Details

Attachments

(1 file)

We already do this for forms such as "pid 1234". We should handle other forms as well such as pid=1234, this would improve diffing for ion and kgsl.

gralloc could be improved as well, but we need to add the process name to the entry so that the values coalesce properly.

ion-memory diff example:

0.05 MB (100.0%) -- ion-memory
├──-5.91 MB (-10800.00%) ── Homescreen (pid=5518) [21] [-]
├──5.91 MB (10800.00%) ── Homescreen (pid=6051) [21] [+]
├──1.87 MB (3421.43%) ── b2g (pid=5884) [5] [+]
├──-1.82 MB (-3321.43%) ── b2g (pid=5319) [3] [-]
├──0.00 MB (00.00%) -- kworker
│  ├──1.58 MB (2892.86%) ── 0:0 (pid=3891) [+]
│  └──-1.58 MB (-2892.86%) ── 1:1 (pid=4446) [-]
├──-1.58 MB (-2892.86%) ── mdss_fb0 (pid=5327) [-]
└──1.58 MB (2892.86%) ── mdss_fb0 (pid=5892) [+]

kgsl-memory diff example:

0.09 MB (100.0%) -- kgsl-memory
├──10.42 MB (11600.00%) -- b2g (pid=5884)
│  ├───3.98 MB (4426.09%) ── egl_image [11] [+]
│  ├───3.16 MB (3521.74%) ── egl_surface [2] [+]
│  ├───2.02 MB (2247.83%) ── gl [11] [+]
│  ├───0.63 MB (695.65%) ── command [10] [+]
│  ├───0.61 MB (682.61%) ── any(0) [18] [+]
│  ├───0.02 MB (21.74%) ── texture [5] [+]
│  └───0.00 MB (04.35%) ── arraybuffer [+]
└──-10.33 MB (-11500.00%) -- b2g (pid=5319)
   ├───-3.95 MB (-4395.65%) ── egl_image [10] [-]
   ├───-3.16 MB (-3521.74%) ── egl_surface [2] [-]
   ├───-2.02 MB (-2247.83%) ── gl [11] [-]
   ├───-0.61 MB (-682.61%) ── any(0) [18] [-]
   ├───-0.56 MB (-626.09%) ── command [9] [-]
   ├───-0.02 MB (-21.74%) ── texture [5] [-]
   └───-0.00 MB (-4.35%) ── arraybuffer [-]

gralloc diff example:


0.05 MB (100.0%) -- gralloc
├──-5.91 MB (-10800.00%) ── pid(5518)/buffer(width=256, height=256, bpp=4, stride=288) [21]
├──5.91 MB (10800.00%) ── pid(6051)/buffer(width=256, height=256, bpp=4, stride=288) [21]
├──3.35 MB (6120.09%) -- pid(5884)
│  ├──3.13 MB (5718.75%) ── buffer(width=480, height=854, bpp=4, stride=480) [2] [+]
│  ├──0.16 MB (301.34%) ── buffer(width=480, height=45, bpp=4, stride=480) [2] [+]
│  └──0.05 MB (100.0%) ── buffer(width=203, height=32, bpp=4, stride=224) [2] [+]
└──-3.29 MB (-6020.09%) -- pid(5319)
   ├──-3.13 MB (-5718.75%) ── buffer(width=480, height=854, bpp=4, stride=480) [2] [-]
   └──-0.16 MB (-301.34%) ── buffer(width=480, height=45, bpp=4, stride=480) [2] [-]
Depends on: 1056962
This patch normalizes all pid entries of the form /pid.[number]/, which matches "pid 123", "pid=1234", etc. It applies the pid normalization to the path as well.
Attachment #8477579 - Flags: review?(n.nethercote)
Example output with the same memory-reports in comment 0:

0.05 MB (100.0%) -- gralloc
└──0.05 MB (100.0%) ── pid(NNN)/buffer(width=203, height=32, bpp=4, stride=224) [2]

0.05 MB (100.0%) -- ion-memory
├──0.00 MB (00.00%) -- kworker
│  ├──1.58 MB (2892.86%) ── 0:0 (pid=NNN) [+]
│  └──-1.58 MB (-2892.86%) ── 1:1 (pid=NNN) [-]
└──0.05 MB (100.0%) ── b2g (pid=NNN) [5]

0.09 MB (100.0%) -- kgsl-memory
└──0.09 MB (100.0%) -- b2g (pid=NNN)
   ├──0.06 MB (69.57%) ── command [10]
   └──0.03 MB (30.43%) ── egl_image [11]
Comment on attachment 8477579 [details] [diff] [review]
Normalize pid values when diffing

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

Thanks!

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +822,5 @@
>      // Strip out some non-deterministic stuff that prevents clean diffs --
>      // e.g. PIDs, addresses, null principal UUIDs. (Note that we don't strip
>      // out all UUIDs because some of them -- such as those used by add-ons --
>      // are deterministic.)
> +    let pidRegex = /pid(.)\d+/g;

/pid([ =])\d+/g ?
Attachment #8477579 - Flags: review?(n.nethercote) → review+
Comment on attachment 8477579 [details] [diff] [review]
Normalize pid values when diffing

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

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +822,5 @@
>      // Strip out some non-deterministic stuff that prevents clean diffs --
>      // e.g. PIDs, addresses, null principal UUIDs. (Note that we don't strip
>      // out all UUIDs because some of them -- such as those used by add-ons --
>      // are deterministic.)
> +    let pidRegex = /pid(.)\d+/g;

Yeah, it seems better to narrow the scope. I'll update it.
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ebeaf707ed6b
Assignee: nobody → erahm
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/ebeaf707ed6b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.