1038921 - Extend system memory reporter to report kgsl memory

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Eric Rahm [:erahm]]

It looks like we can break out kgsl memory usage per-process by looking at the |/d/kgsl/proc/<pid>/mem| file on b2g devices. The total should roughly equal the |gralloc| entry for the system process, but it would be nice to have a cleaner breakdown.

Proposed format is something like this:

152.72 MB (100.0%) -- kgsl
├───88.94 MB (58.24%) -- pid=123
│   ├──86.49 MB (56.63%) ── texture
│   └───2.45 MB (01.61%) ── arraybuffer

Comment 1

[Eric Rahm [:erahm]]

Attachment: Extend system memory reporter to report kgsl memory

This patch adds kgsl memory reporting to the SystemMemoryReporter.

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 8459860 [details] [diff] [review]
Extend system memory reporter to report kgsl memory

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

Thanks!

::: xpcom/base/SystemMemoryReporter.cpp
@@ +869,5 @@
> +  struct DirGuard
> +  {
> +    DirGuard(DIR* aDir) : dir(aDir) {}
> +    ~DirGuard() { closedir(dir);  };
> +    DIR* dir;

mDir

@@ +870,5 @@
> +  {
> +    DirGuard(DIR* aDir) : dir(aDir) {}
> +    ~DirGuard() { closedir(dir);  };
> +    DIR* dir;
> +  };

RAII is great! But we usually call classes like this Auto<whatever>. Though MFBT uses Scoped<whatever>, for some reason. Anyway, use one of those.

@@ +876,5 @@
> +  struct FileGuard
> +  {
> +    FileGuard(FILE* aFile) : file(aFile) {}
> +    ~FileGuard() { fclose(file); }
> +    FILE* file;

mFile

@@ +890,5 @@
> +    //   gpuaddr useraddr size id  flags type usage sglen
> +    //   hexaddr hexaddr  int  int str   str  str   int
> +    // We care primarily about the usage and size.
> +
> +    // For simplicity numbers will be uint64_t, strings 15 chars max.

Do you know that 15 is enough? It's not clear from the comment. If not, maybe make it a bit higher, e.g. 63?

@@ +951,5 @@
> +                    flags, type, usage, &sglen) == kNumFields) {
> +        nsPrintfCString entryPath("kgsl-memory/%s/%s", procName.get(), usage);
> +        REPORT(entryPath,
> +               size,
> +               NS_LITERAL_CSTRING("A kgsl graphics memory allocation"));

Nit: Period at end of the sentence.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> Comment on attachment 8459860 [details] [diff] [review]
> RAII is great! But we usually call classes like this Auto<whatever>. Though
> MFBT uses Scoped<whatever>, for some reason. Anyway, use one of those.

I'll do AutoDir/AutoFile (I'm assuming that's what you meant, not AutoDirGuard). MFBT literally has a Scoped<T> class which I _could_ use, but it seems like overkill.

> Do you know that 15 is enough? It's not clear from the comment. If not,
> maybe make it a bit higher, e.g. 63?

The longest I've found is 15, but it's probably better to err on the side of caution.

Comment 4

[Nicholas Nethercote [inactive]]

> I'll do AutoDir/AutoFile (I'm assuming that's what you meant, not AutoDirGuard).

Yes!

Comment 5

[Eric Rahm [:erahm]]

try: https://tbpl.mozilla.org/?tree=Try&rev=6380b5346ba8

Comment 6

[Eric Rahm [:erahm]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0b1d53fd456b

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0b1d53fd456b

Comment 8

[Eric Rahm [:erahm]]

[Blocking Requested - why for this release]: Nom'ing for 2.0+, this should be useful for identifying and working on graphics memory regressions.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8c695bbb82e9

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Backed out along with bug 1038888 for bustage.
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/fbb3b8be8f6c

https://tbpl.mozilla.org/php/getParsedLog.php?id=44563561&tree=Mozilla-B2g32-v2.0

Comment 11

[Eric Rahm [:erahm]]

Attachment: b2g_2.0_kgsl.patch

Patch for the b2g 2.0 branch, adds missing function not on 2.0.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/be79dc968dd6