1373057 - Diffs should remove the scripts count

Status: RESOLVED FIXED

(Toolkit :: about:memory, enhancement)

Description

[Andrew McCreight [:mccr8]]

When I diff two about:memory reports I see stuff like this:

│   │  │   ├──0.03 MB (-6.33%) ── source(scripts=154, <non-notable files>)/misc
│   │  │   ├──-0.03 MB (06.13%) ── source(scripts=149, <non-notable files>)/misc
│   │  │   ├──-0.02 MB (05.71%) ── source(scripts=139, <non-notable files>)/misc
│   │  │   └──0.02 MB (-5.67%) ── source(scripts=138, <non-notable files>)/misc [3]

The scripts=154 should be turned into scripts=NNN or something.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Normalize script source counts in about:memory diffs.

We care about the memory used by these scripts, not by the precise
count of them.

Comment 2

[Andrew McCreight [:mccr8]]

There are other things besides <non-notable files>, so I just match on "source(scripts=\d+, ".

Comment 3

[Eric Rahm [:erahm]]

Comment on attachment 8877774 [details] [diff] [review]
Normalize script source counts in about:memory diffs.

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

Looks good, but can you please update the tests? See bug 1142174 for an example.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Normalize script source counts in about:memory diffs.

We care about the memory used by these scripts, not by the precise
count of them.

Comment 5

[Eric Rahm [:erahm]]

Comment on attachment 8877781 [details] [diff] [review]
Normalize script source counts in about:memory diffs.

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

Almost there, you need to update the test (test_aboutmemory3.xul) as well. You can run it with something like:

> ./mach test toolkit/components/aboutmemory/tests/

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #5)
> Almost there, you need to update the test (test_aboutmemory3.xul) as well.

FWIW, I ran it locally and it passed.

Comment 7

[Andrew McCreight [:mccr8]]

Maybe diffs of 0 are not included under explicit, but they are for the other subtrees?

Comment 8

[Eric Rahm [:erahm]]

Comment on attachment 8877781 [details] [diff] [review]
Normalize script source counts in about:memory diffs.

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

Hmm okay, might make sense to do a non-zero diff, but eh.

Comment 9

[Nicholas Nethercote [inactive]]

(In reply to Andrew McCreight [:mccr8] from comment #7)
> Maybe diffs of 0 are not included under explicit, but they are for the other
> subtrees?

My guess would be that diffs of 0 are included for degenerate (single-node) trees, but not as non-root nodes in proper trees.

Comment 10

[Andrew McCreight [:mccr8]]

I ran the tests this should affect locally, so I don't think it needs a try run.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2674fc87b8d6
Normalize script source counts in about:memory diffs. r=erahm

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2674fc87b8d6