605362 - Add (coarse-grained) memory reporters for shmem

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I propose reporting

 shmem/allocated
 shmem/mapped

(lumping all the different shmem types, basic/sysv/android together) and the specific uses as, e.g.

 shmem/imagesurface
 shmem/ximage
 shmem/cgbitmap

Vlad, does that follow the spirit of the about:memory display?  Not sure how the various counts are expected to add up.

Let's have this bug cover the gross shmem usage, and report the breakdown in followups.  (This is most useful for mobile, and all the shmem we allocate there atm is for gfxSharedImageSurfaces.)

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

That looks fine -- the about:memory detailed display doesn't add up to any of the numbers, you sort of need a decoder ring to figure out which things are supposed to add up to any other things.  But that's fine, I'd rather have as much data as we can in there and do the analysis separately if we run into memory-related problems.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 1: Allocate page-aligned shmem segments in ShmImage, to match other allocators which more honestly report address space and system mem taken by alloc

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 2: Track allocated sizes (in the allocating process) in all shmem backends

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 3: Add MemoryReporters for shmem.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 4: Notify shmem MemoryReporters from all shmem backends

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The way these patches work is, shmem/allocated reports the total shmem bytes actually allocated (by the system interfaces) by a process.  shmem/mapped returns the total bytes mapped, which might be more, less, or the same as the amount allocated (though in our usage so far, mapped >= allocated).  This allows us to see what has been allocated in the content process vs. chrome.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 484239 [details] [diff] [review]
part 3: Add MemoryReporters for shmem.

er, 'static bool registered;' is missing a = false?

Comment 8

[Joe Drew (not getting mail)]

Comment on attachment 484237 [details] [diff] [review]
part 1: Allocate page-aligned shmem segments in ShmImage, to match other allocators which more honestly report address space and system mem taken by alloc


>+/*static*/ size_t
>+SharedMemory::PageAlignedSize(size_t aSize)
>+{
>+  size_t pageSize = SystemPageSize();
>+  size_t nPagesNeeded = int(ceil(double(aSize) / double(pageSize)));

= size_t(ceil(...))?

Comment 9

[Joe Drew (not getting mail)]

Comment on attachment 484238 [details] [diff] [review]
part 2: Track allocated sizes (in the allocating process) in all shmem backends

Why not track/store alloc size in SharedMemory? We do something similar in gfxASurface, fwiw.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #8)
> Comment on attachment 484237 [details] [diff] [review]
> = size_t(ceil(...))?

Oop, yep.

(In reply to comment #9)
> Comment on attachment 484238 [details] [diff] [review]
> part 2: Track allocated sizes (in the allocating process) in all shmem backends
> 
> Why not track/store alloc size in SharedMemory? We do something similar in
> gfxASurface, fwiw.

Well, we're tracking allocs in SharedMemory.  I thought about making mAllocSize a member of SharedMemory, but the only thing I believe it buys us is being able to call DestroyedShmem() from ~SharedMemory.  I could make that change if you want.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 5: Centralize more of the accounting

Comment 12

[Joe Drew (not getting mail)]

Comment on attachment 485471 [details] [diff] [review]
part 5: Centralize more of the accounting

So we're no longer going to report things as unmapped or destroyed until the destructor is called?

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We don't (currently) support unmapping or destroying SharedMemorys separate from ~SharedMemory().

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Requesting blocking because it's going to be hard to get memory usage info from users in the field without these patches.  Technically approval? is probably more appropriate.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/f5e82d1422e6
http://hg.mozilla.org/mozilla-central/rev/62bbf0aed452
http://hg.mozilla.org/mozilla-central/rev/e25e8b0bb4d0
http://hg.mozilla.org/mozilla-central/rev/e390c6cf96b1
http://hg.mozilla.org/mozilla-central/rev/ffb9040a013b
http://hg.mozilla.org/mozilla-central/rev/0aa98eae87ed