My personal opinion is the following:
I agree with haraken@ that tracking this at the client-side is hard to maintain. Every now and then we have new things in the codebase that add new use of shared memory, and we can't rely on us watching the codebase (the last example that comes to my mind is ccrev.com/1410213004 which I learned about by accident). I'd love to have this kind of things being caught by telemetry tests.

I agree with reveman@ that tracking at the base level might be tricky from a performance viewpoint. However, I think there is room for doing something clever and non perf-intrusive here.

This is my proposal:
SharedMemory uses mmap (or equivalent) that is a syscall. syscalls are quite expensive in general, mmap in particular (*). Can we figure out some tracking which is faster than a syscall, so won't be noticed?
To be clear, I think that taking a mutex and populating some hashtable to keep track of the mmaps is a no-go (read: still too slow).

My concrete proposal is: can we try a lightweight log append approach? More in detail, say that we have something like this:

class SharedMemoryLog {
  struct SharedMemoryEvent {
    enum { MAP, UNMAP, CHANGE_FLAGS } type;
    uintptr_t start_addr, end_addr;
  } 

  SharedMemoryEvent circular_buffer[SOME_FIXED_SIZE];

  // This is called from fast-paths by base::SharedMemory
  public static void LogSharedMemoryEvent(...) {
    SharedMemoryEvent* evt = atomic_whatever(circular_buffer);
    evt.type = .... // populate fields
  }

  // This is called by memory-infra and does the number crunching
  public set<SharedMemoryInfo> ProcessLogAndGetMmaps() { 
    // iterate over the log, apply that on top of the last known state
    // and compute the new situation. 
  }
}

In essence what we'd add is an atomic operation to base::SharedMemory. My personal opinion (but we'd need data to confirm) is that an atomic op is << even just the syscall prologue and wouldn't be noticed.
The other things we need to confirm is: how frequently is SharedMemory() called? In other words, how big that ring buffer would need to be? How frequently we'd need to process it out of band (read: outside of mmap ops)?
My hope/speculation here is that a smallish (few k elements) buffer should be enough if we flush it on a idle callback (and on memory dumps).    

So I think the way to make this forward is:
1) Check whether we agree on this plan.
2) Check whether base OWNERs would be fine with that.
3) Do some preliminary perf test to make test that my speculations (both on the perf intrusiveness and circular buffer size) are reasonable.

If 1-3 fly, then we'd need to make this into code (and deal with trickier aspects like double-counting, but let's keep that separate). I personally don't have  bandwidth for this in this Q2 due to the tracing rearchitecture going on, but maybe we can figure out some other owner for this (or just keep it for Q3)?


(*) I can't speak for all platforms, but on Linux/Android/CrOS at least that is a pretty expensive syscall as that takes (inside the kernel) the map_sem, which in concrete words menans that is serialized with any other page fault happening within the same process. On top of that, I think that in all cases mmap causes a TLB flush, which is an extra perf hit on top of the syscall time.

I agree with the plan!

sgtm

reveman@: Do you have any person who has a bandwidth to work on this?

Assigning to me. Will try to find time to implement.

primiano@, what do you mean by "an idle callback" specifically? As for the frequency of mmap() and munmap(), it seems that it could be 300+ between OnMemoryDump() calls. Also, we have --enable-memory-benchmarking flag, which disable periodic dumps. Computing stats only on OnMemoryDump() wouldn't be an option. To make the proposal workable, we need to have another way to flush the ring buffer and update the stats.

> primiano@, what do you mean by "an idle callback" specifically?
I was thinking to RequestIdleTaskRunner, but that is renderer only.
Maybe we can PostTask(ProcessRingBuffer) every time the distance between writer and reader becomes ~ > 1/4 th ring buffer size, so that if OnMemoryDump is fast enough, it's all good. If not a deferred task will flush it.

Specifically, I took 1/4th out of the blue, we need to fine tune it with some experiments. As long as we have a way to tell whether the writer passed the reader it's fine (which we can do by kust keeping the read/write pointers uint64 without ever wrapping them % BUF_SIZE. Do the % only when dereferencing the items in the buf)

> Also, we have --enable-memory-benchmarking flag, which disable periodic dumps
That flag should go away soon, as soon as the TraceConfig is properly plumbed through telemetry. Plz don't rely on that.

My current understandings are:

1. We need to track all mmap/munmap events to make the stats be consistent even when we don't enable tracing. Otherwise, there would be unbalanced mmap (or munmap) events. Numbers calculated from these events won't make sense.
2. We need to post a task when the ring buffer doesn't have enough space (1/4 th size, per comment #7).
3. If 1. and 2. are true, we need to do more than just an atomic operation on every mmap/munmap events.
4. 1-3 means that we will add some extra costs even when tracing is disabled.

Are these correct? I think it's fine to add some extra cost when we enable tracing, but I'm not sure it's acceptable to add costs for normal paths.

Right my proposal was that 1 out of ~hundreds mmap will also cause a PostTask.
Can we do a prototype (just a counter in shared_memory which posts a dummy task every ~100 calls) and run through trybots / analyze locally the avg+stddev of the call duration?

I wrote a prototype CL and started a tryjob.
https://codereview.chromium.org/1941683002/

On my local machine,
- Need more than 256 slots to avoid running out the ring buffer on heavy sites.
- # of events could be 6000+ on heavy sites (e.g. youtube).
- There aren't noticeable regression.

Is there any progress on hooking shared memory?

I have a local CL which works in most cases but seems to have bugs on some edge cases. Since mmap() is a heavy syscall we might want to use a lock.

hajimehoshi@: could you take over this as we discussed offline?

> I have a local CL which works in most cases but seems to have bugs on some edge cases.

I'd like to read that CL.

Sorry, I lost the CL and didn't upload it anywhere :( It was similar to comment 10 but it seems that it's gone as well...

I have confirmed "global/foobarbaz" is created in ProcessMemoryDump::CreateSharedGlobalAllocatorDump or CreateWeakSharedGlobalAllocatorDump, but how can I see them on chrome://tracing? Thanks.

> I have confirmed "global/foobarbaz" is created

I mean MemoryAllocatorObjects with names begenning "global/".

> > I have confirmed "global/foobarbaz" is created

> I mean MemoryAllocatorObjects with names begenning "global/".

Oops, MemoryAllocatorDump objects...

global/ dumps are invisible. the way it works is that you have two "standard" dump that link (via AddOwnershipEdge) to the global one. Essentially the global is only used to to the binding.

See https://cs.chromium.org/chromium/src/content/common/host_shared_bitmap_manager.cc?rcl=0&l=179 for an example

primiano@: Thank you for replying (and sorry for the late reply). IIUC, this seems to have nothing to do with SharedMemory object.


During writing a prototype CL (https://codereview.chromium.org/2535213002/), we realized that shared memory file is duplicated by dup(2) (in SharedMemory::ShareToProcessCommon) and we need to consider them. This would require not only simply incrementing and/or decrementing memory usage of SharedMemory but also managing chunks and counting how many times the chunks are duped. primiano@, is this correct?

So first of all one clarification: when we use SharedMemroy there are two use cases: 1) we share anonymous memory cross-process (this is very common and this consumes "dirty" memory). 2) We share files (in most cases read-only, which consumes "clean" memory). I imagine here we care only about 1). can you confirm?

>  we realized that shared memory file is duplicated by dup(2) (in SharedMemory::ShareToProcessCommon) and we need to consider them

Can you expand when you say " we need to consider them".
file descriptors are capabilities. We dup() them so that we can give a handle to the other process, but they refer to the same resource. I am assuming (but I might be wrong here) that when we dup() fds we don't use both copy in the same process.

Now, the real problem here is: when a shared memory handle is used on both sides (say by browser and renderer) you need to tell memory-infra about the aliasing to avoid counting them twice. This is what we do today in the discardable memory dump provider.
In order to do this, you need some kind of unique ID that you can refer to on both sides, so you can reconcile the duplication (really we should take strong inspiration from what ssid did in the discardable dumper).

Now the challenge is: what is such ID, unique for each mapping, but shared between both processes?
- You cannot use the mapped_file, because (I think) that will be an empty string for anonymous mappings (plz check).
- You cannot use the fd, because the fd number will be different on both sides, even if they refer to the same resource.

At this point the options left are:
- You can look if we carry some form of unique ID over IPC when we handle out the shared memory handle (don't think so)
- You can fstat(fd): both dup()-ed fd should refer to the same inode and, I think, that should be unique even across processes.  If this works I am not sure though about how to do this on mac or windows.

Thank you for explanation!

As primiano@ and I have chatted online. My understanding is:

There are classes which use SharedMemory and some classes do OnMemoryDump and the other don't. The former classes like DiscardableMemory add ownership relationship to "global" dump and already avoid duplicated counts (see CreateSharedGlobalAllocatorDump). As for the latter, there are many such classes and it is almost impossible to add OnMemoryDump to all of them. Instead, we could add dumps inside SharedMemory class.

1. Add conventional dumps for SharedMemory usages with an ID e.g. /sharedmemory/foo.
2. Add an ownership relationship between the dump (1.) and the global dump like [ /sharedmemory/foo -> /global/sharedmemory/X ] where X is an invisible and identical for each mapping.
3. Add an ownership relationship between existing global dumps like DiscardableMemory and global dump like [ /global/discardable/Y -> /global/sharedmemory/Y ]

With 2 and 3, shared memory usage is correctly counted without duplication.

We would be able to get total memory usage of SharedMemory on memory infra.

primiano@, is this correct?

The plan sounds reasonable to me.

Yup SGTM.
So the key is: when you have shared memory shared by multiple processes (say browser and renderer) this is the way to properly dump them in memory-infra:

*each process* has to create two dumps (I know, this is the part which is a bit contra-intuitive):
1) A standard dump via CreateAllocatorDump (what you called /sharedmemory/foo)
2) A global dump via CreateSharedGlobalAllocatorDump (what you called /global/sharedmemory/X)
3) An edge from 1 to 2 

1. Is visible in the UI (under each process, see discardable as an example) and can have a different name in each process, 2 is invisible in the ui. 
2 is necessary to establish the global binding. The fundamental bit here is that both browser and renderer must use the same guid when creating 2.

So you want to end up in a situation where you have:
/sharedmem/foo  -> /global/GUIDX  # From process 1
/sharedmem/fooz -> /global/GUIDX  # From process 2

At this point since GUIDX is identical, memory-infra will infer that /sharedmem/foo and /sharedmem/fooz alias the same object and will do the right discounting.

In turn then we have to figure out extra edges from classes, like discardable, that today are based on SharedMemory (but this I suggest to be a follow-up cl) to avoid double counting between discardable and sharedmemory.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c8897721e2df9a0ec3677d0a13d677c457e5e1e

commit 6c8897721e2df9a0ec3677d0a13d677c457e5e1e
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Mon Dec 19 06:49:52 2016

Bug fix: TranferBufferManager's SharedMemory memory usage reporting was wrong

TranferBufferManager takes buffers whose backing store might or might
not be SharedMemory, but accumulates shared memory usage no matter what
the buffer is. This CL fixes this to make TranferBufferManager count
shared memory usage only when the buffer's backing store is
SharedMemory.

BUG= 604726 
TEST=gpu_unittests --gtest_filter=TransferBufferManagerTest.*
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2575803002
Cr-Commit-Position: refs/heads/master@{#439419}

[modify] https://crrev.com/6c8897721e2df9a0ec3677d0a13d677c457e5e1e/gpu/command_buffer/common/buffer.cc
[modify] https://crrev.com/6c8897721e2df9a0ec3677d0a13d677c457e5e1e/gpu/command_buffer/common/buffer.h
[modify] https://crrev.com/6c8897721e2df9a0ec3677d0a13d677c457e5e1e/gpu/command_buffer/service/transfer_buffer_manager.cc

One thing that makes things complicated is SharedMemory::TakeHandle. This function is used only at GpuMemoryBufferImplSharedMemory::CreateGpuMemoryBuffer. This gives the handle ownership to the caller, and makes a called SharedMemory empty. Unfortunately such SharedMemory is not dumped correctly in our way because SharedMemoryTracker::usages' key is SharedMemory* and we assume that the ownership is not taken during recording implicitly. This is one of the cases that SharedMemoryHandle is kept instead of SharedMemory, and there might be other cases.

To record SharedMemory usages correctly for such cases, I'm thinking of changing SharedMemoryTraker::usages' key from SharedMemory* to SharedMemoryHandle, I'm not sure this works correctly though. Is there any better ideas?

I found SharedMemoryHandle's lifetime has nothing to do with mmap-ed memory's lifetime, so my idea above would not work. If I understand correctly:

* Basically, SharedMemory manages SharedMemoryHandle (file descriptor) and mmap-ed memory.
* SharedMemory::Close closes its handle.
* SharedMemory::Unmap unmaps its memory.
* Close can be called without unmapping. Closing a handle has nothing to do with the lifetime of mmap-ed memory.
* In our plan, SharedMemoryHandle is used to generate unique identifier, but there are some cases that SharedMemory doesn’t have its SharedMemoryHandle due to Close (e.g. ChildSharedBitmapManager::AllocateSharedBitmap).
* When SharedMemoryTracker::OnMemoryDump is called, the recorded SharedMemory might not have its handler, which means ID can’t be generated correctly when OnMemoryDump.
* Unique ID is generated with fstat. This means IDs can be generated only when calling fstat is permitted. Regardless of performance, it would be ideal if we can do SharedMemory::Map and generating ID at the same time, but it is impossible since fstat is not-always permitted (e.g. BrowserChildProcessHostImpl::CreateMetricsAllocator)

Anyway, in our current implementation, memory usages are not recorded. primiano@, what do you think?

Your considerations in #30 seem correct. Some though in this regard:

>  When SharedMemoryTracker::OnMemoryDump is called, the recorded SharedMemory might not have its handler, which means ID can’t be generated correctly when OnMemoryDump.

This is the part I am not sure I follow. we definitely have the handle at the time the mmap happens. I though we did save the the handle in the tracker, and use the absolute address to index mmaps, in a hashtable like this:

mapped address -> (HandleID, size, any other useful information)

so that:
 - when the memory is unmapped we can directly find and remove it from the hashtable
 - when we OnMemoryDump we have the information about the handle so we can emit the correct global dump ID that will match on the various processes.

> * Unique ID is generated with fstat. This means IDs can be generated only when calling fstat is permitted. Regardless of performance, it would be ideal if we can do SharedMemory::Map and generating ID at the same time, but it is impossible since fstat is not-always permitted (e.g. BrowserChildProcessHostImpl::CreateMetricsAllocator)

What do you mean with "not always permitted?"

Some thoughts here:
1. Did you check how slow fstat actually is? AFAIK is a metadata only operation and, for recently touched files that are in pagecache, is very ulikely to do any I/O for real. 
2. If there are performance considerations an alternative would be:
  - At the time we do the mmap() just record the start address and size in the hashtable but not the inode information. That means that, if an OnMemoryDump comes immeditaly at this point, we should skip the entry (i.e. at this point the entry is still incomplete)
  - As soon as we do a mmap we PostTask on the blocking pool the fstat operation and we add the information to the hashtable in a deferred way.

In all honestly 2) might be a bit overkill and, if I have to make an educated guess, I'd bet that doing the PostTask is actually slower than the fstat operation itself.

Thank you for the advice!

>  we definitely have the handle at the time the mmap happens

I think that's correct.

> mapped address -> (HandleID, size, any other useful information)

This seems to work correctly.

> What do you mean with "not always permitted?"

As we chatted, I saw an error but this was because of ThreadRestrictions::AssertIOAllowed() which I newly added. Probably I can remove that, so that fstat can be called anywhere :-)

OK, I'll fix the CL with your suggestion to get a shared memory's unique ID whenever mmap-ing and measure the performance again.

Thank you very much!

> As we chatted, I saw an error but this was because of
> ThreadRestrictions::AssertIOAllowed() which I newly added. Probably I can
> remove that, so that fstat can be called anywhere :-)

That just does IO without telling chrome that it's happening. I don't think that's the appropriate fix. Rather, places that want to do IO should allow it with ScopedAllowIO, and that makes it clear that IO will happen, and we can determine if it's a good idea. My understanding from conversations on chromium-dev is that any IO can end up blocking on the OS, even tho it looks cheap, so you don't want to do it from low latency threads for instance.

I found another problem: On Windows, I thought we could get the file information with GetFileInformationByHandle (counterpart of fstat), but it seems impossible. On Windows, SharedMemory::mapped_file_ is a HANDLE for a file mapping object, not a file. This is created with CreateFileMapping without indicating a file system's file. If I understand correctly, such file mapping objects' back end is invisible.

Instead of using fstat, I'm thinking of using an atomic integer counter to generate IDs and have handlers have those IDs. An ID would consist of the counter value and the creator processes' 'tracing process ID' (*1). In the case of closing a handle, we also need to have SharedMemory have the same ID. This would no longer require IO like fstat. I think this works because when sharing a SharedMemory between processes, SharedMemoryHandle is always shared (*2). Anyway, measurement of performance is required.

(*1) MemoryDumpManager::GetTracingProcessId()
(*2) There is one exception, ServiceProcessUtil, which uses SharedMemory::Open to indicate a file name and shared that by the name. I don't think this is a big memory consumer and not a big problem.

>  Rather, places that want to do IO should allow it with ScopedAllowIO, and that makes it clear that IO will happen, and we can determine if it's a good idea.

Thank you for the advice, and I'd do that if we continue fstat way.

Ugh, MemoryDumpManager::GetTracingProcessId() is available only when tracing is working. Need to think another way.

Would GetUniqueIdForProcess() work?

The unique process ID is available only on browser and the children does not know the child process ids. This id should not be sent to the child processes for security reasons.
If the SharedMemoryHandle is unique and all the processes know the handle for a memory segment, then you could just create the global dump with the handle, and do not need process ids at all.
Why do you need the creator process ID at all?

+1 to what ssid said. If you have a global unique handle (the inode number) there should be no need for any process id. We use the process id in other systems and (e.g. Discardable) because there is no other primary key other than (process id, local segment id), but this should not be the case here 

> Why do you need the creator process ID at all?

It's because IIUC it seems impossible to get inode-like ID from a mapping object on Windows as I described at #34. If we need to introduce a system to generate unique identifiers for mapping objects on Windows, I thought we could use that on other platforms including Posix.

I found that a mapped object is always created with a unique name, and I can use this on Windows for identifiers. It looks like there is no way to obtain a name from HANDLE of a mapped object, but I think I can embed the name to SharedMemoryHandle struct. OK, let's continue fstat way.

Re #33 
> That just does IO without telling chrome that it's happening. ... My understanding from conversations on chromium-dev is that any IO can end up blocking on the OS, even tho it looks cheap, so you don't want to do it from low latency threads for instance.

TL;DR Dana: I think we are already in this state (i.e. doing fstat() during SharedMem creation assuming that they will be fast because shmem is backed by pagecache and not by a real filesystem)

Here's my though on the matter: in theory fstat is a I/O-related syscall, and as Dana says could introduce latency. The workaround here would be to defer (i.e. PostTask) the fstat call to get the inode number of a blocking pool.
However: my theory here is that effectively a PostTask would be slower than the fstat() call in most cases (hajime, can you locally add some TRACE_EVENT around the fstat to see how expensive is that in reality?).
Here's my line of reasoning: there are two cases for SharedMemory:
(i) handing file-mapped memory to other processes that can't open() because of sandboxing (e.g. when loading the V8 snapshot or ICU data)
(ii) create anonymous memory and share it between processes for fast IPC (e.g., gpu transfer buffers and such)

(i) is extremely rare, and when happens we must be already on a thread that supports I/O, otherwise we wouldn't be able to open()+mmap() the file in the first place. So there is no harm here in doing a fstat() here.
(ii) is the most frequent one. In this case we end up doing I/O operations on shmem which is NOT a real filesystem and is backed purely by the pagecache. So syscalls like fstat don't hit the disk and just grab some metadata from the pagecache.

After writing the above, I scraped the existing code and realized that we are already in this state, and we are already making this assumption.
If you open shared_memory_posix.cc, you will see:

bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
  ...
  // This function theoretically can block on the disk, but realistically
  // the temporary files we create will just go into the buffer cache
  // and be deleted before they ever make it out to disk.
  base::ThreadRestrictions::ScopedAllowIO allow_io;
  ...
  return PrepareMapFile(...)

and in turn
bool PrepareMapFile(...)
  ...
  base::ThreadRestrictions::ScopedAllowIO allow_io;
  ...
   if (fstat(readonly_fd.get(), &readonly_st))
     NOTREACHED();

So we already are in the state where SharedMemory has ScopedIOAllow and actually does a fstat call already.
I hope this addresses Dana's concerns.

In theory we could piggy back on that fstat() call in PrepareMapFile(). In practice, though, I am not sure if it's worth it. That code is behind #ifdef !ANDROID because android uses a different path for sharedmemory based on ashmem. So that would probably create more awkwardness for the plumbing than making a further fstat call itself in the right place.

----
Slightly OT: that comment "and be deleted before they ever make it out to disk." seems outdated to me. From what I see in the code these days we don't link the tmp file in the real filesystem anymore and use directly the /dev/shm driver, which is never backed by a block device. So I think that refers to some older code that was creating actual files, before we started using /dev/shm.
----

I'm not trying to argue against doing fstat, just against removing AssertIOAllowed. We want a world where all uses of IO are inside ScopedIOAllowed. Then the task scheduler can use that information to know that if a thread blocks at all, its due to external forces, and it can use that to schedule tasks accordingly.

I measured the performance of the CL ( https://codereview.chromium.org/2654073002) with TRACE_EVENT around fstat. The result (JSON data for chrome://tracing) is:
https://drive.google.com/file/d/0BwW8PrCcts4WTGdXa29vcmN6QkU/view?usp=sharing
The test is simple: I just browsed google.co.jp and scrolled it. It looks like fstat only takes about 5 micro seconds.

By the way, I found PRESUBMIT.py doesn't allow ScopedAllowIO for new code:
https://cs.chromium.org/chromium/src/PRESUBMIT.py?l=181

As primiano@ explained, fstat wouldn't cause I/O problems. I'd want to just leave a comment before fstat to explain this situation. Dana, what do you think?

> I measured the performance of the CL ( https://codereview.chromium.org/2654073002) with TRACE_EVENT around fstat. The result (JSON data for chrome://tracing) is:
> https://drive.google.com/file/d/0BwW8PrCcts4WTGdXa29vcmN6QkU/view?usp=sharing
> The test is simple: I just browsed google.co.jp and scrolled it. It looks like fstat only takes about 5 micro seconds.

The trace event's category name is 'shared_memory' and event name is 'GetUniqueId'.

> As primiano@ explained, fstat wouldn't cause I/O problems. I'd want to just
> leave a comment before fstat to explain this situation. Dana, what do you
> think?

I don't understand what we're gaining by hiding the fact that IO is happening. Yes ScopedAllowIO should not be used on non FILE thread generally. There is a whitelist to add things to if it should be allowed to do IO. The presubmit requires you to think about what you're doing.

Ah, I couldn't realize I could add that to the whitelist. Thanks.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79

commit 2acea43a7272838a75e4c6b7e5473dd6f9c4aa79
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Wed Mar 08 08:55:37 2017

base: Introduce SharedMemoryTracker for POSIX (but not macOS)

This CL introduces SharedMemoryTracker to measure memory usage of
SharedMemoy correctly and report them to memory-infra.
SharedMemoryTracker records the memory usage when mmap-ing and
when unmmap-ing, and reports them when OnMemoryDump is called.
This enables to help us know SharedMemory usage correctly.

This is a part of a CL (https://codereview.chromium.org/2535213002/) for
SharedMemory usage measuring.

BUG= 604726 

Review-Url: https://codereview.chromium.org/2654073002
Cr-Commit-Position: refs/heads/master@{#455406}

[modify] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/PRESUBMIT.py
[modify] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/base/BUILD.gn
[modify] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/base/memory/shared_memory.h
[modify] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/base/memory/shared_memory_posix.cc
[add] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/base/memory/shared_memory_tracker.cc
[add] https://crrev.com/2acea43a7272838a75e4c6b7e5473dd6f9c4aa79/base/memory/shared_memory_tracker.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c6261c7a497796269a6befd9efd7dfca9957fe8

commit 7c6261c7a497796269a6befd9efd7dfca9957fe8
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Wed Apr 19 08:16:52 2017

Reorganize GUIDs for GPU memory buffers

This CL replaces current GUID generators (GetGenericSharedMemoryGUID-
-ForTracing and GetGpuMemoryBufferGUIDForTracing) with new GUID
generators GetSharedMemoryGUIDForTracing and GetGenericSharedGpumemory-
-GUIDForTracing. The formater is used for GPU memory buffer that use
base::SharedMemory, and the latter is not. This fix is required to know
shared memory usages on memory-infra accurately.

BUG= 604726 
TEST=n/a
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2819803002
Cr-Commit-Position: refs/heads/master@{#465529}

[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/cc/raster/staging_buffer_pool.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/cc/resources/resource_provider.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/gpu/ipc/client/gpu_memory_buffer_impl_shared_memory.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/gpu/ipc/client/gpu_memory_buffer_impl_shared_memory.h
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/generic_shared_memory_id.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/generic_shared_memory_id.h
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/gpu_memory_buffer.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/gpu_memory_buffer.h
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/gpu_memory_buffer_tracing.cc
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gfx/gpu_memory_buffer_tracing.h
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gl/gl_image_io_surface.mm
[modify] https://crrev.com/7c6261c7a497796269a6befd9efd7dfca9957fe8/ui/gl/gl_image_shared_memory.cc

The net stack also uses SharedMemory ( Issue 622363 ). Any plan to attribute the usage to net stack's MemoryDumpProvider?

xunjieli@: nothing so far because 1. SharedMemory usages are taken care of SharedMemoryTracker, and 2. we need fix all current ProcessMemoryDump::CreateSharedGlobalAllocatorDump usages but this is not used in the net stack.

s/taken care of/taken care of by/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e370c17ca1827988dd84a545bb728f9b6699d42d

commit e370c17ca1827988dd84a545bb728f9b6699d42d
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Thu May 25 06:12:16 2017

Replace SharedMemory::UniqueID usages with SharedMemoryHandle's guid

Shared memory's unique IDs are required to dump memory usages and before
this CL, files' inode values are used for them on POSIX. The problem was
that this was not portable. This CL replaces the current unique id with
the new shared memory handler's GUID introduced at crrev/2859843002,
which is available on any platforms.

Note that passing GUID across mojo is not implemented so the current
shared memory usages on memory-infra are still not accurate. Mojo fix
will be done in the future.

BUG= 604726 ,  713763 
TEST=n/a

Review-Url: https://codereview.chromium.org/2873433004
Cr-Commit-Position: refs/heads/master@{#474579}

[modify] https://crrev.com/e370c17ca1827988dd84a545bb728f9b6699d42d/base/memory/shared_memory.h
[modify] https://crrev.com/e370c17ca1827988dd84a545bb728f9b6699d42d/base/memory/shared_memory_posix.cc
[modify] https://crrev.com/e370c17ca1827988dd84a545bb728f9b6699d42d/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/e370c17ca1827988dd84a545bb728f9b6699d42d/base/memory/shared_memory_tracker.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e

commit e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Fri May 26 05:49:36 2017

Recode shared memory usage on Windows, macOS and NaCl

Now that shared memory's unique ids are portable, shared memory usages
can be counted. This CL inserts incrementing and decrementing shared
memory usages when map/unmapped.

BUG= 604726 

Review-Url: https://codereview.chromium.org/2867193004
Cr-Commit-Position: refs/heads/master@{#474930}

[modify] https://crrev.com/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e/base/BUILD.gn
[modify] https://crrev.com/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e/base/memory/shared_memory_mac.cc
[modify] https://crrev.com/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e/base/memory/shared_memory_nacl.cc
[modify] https://crrev.com/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e/base/memory/shared_memory_posix.cc
[modify] https://crrev.com/e80c576c7bdcd567ba96a4e1ceb9f845dcf1168e/base/memory/shared_memory_win.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/394636a1dbb10af27400f21b0326daedb3b6fede

commit 394636a1dbb10af27400f21b0326daedb3b6fede
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Wed May 31 07:18:42 2017

Add gpu::BufferBacking::shared_memory_handle()

This CL adds a base::SharedMemoryHandle getter to gpu::BufferBacking.
The getter will be used by gpu::BufferBacking users when OnMemoryDump is
called to create base::SharedMemory dumps.

This is part of work for exposing SharedMemory usages on tracing
memory-infra dumps.

Design doc: https://docs.google.com/document/d/16Mi5_puxKgQ-9IX7ANfbji0WiqBCK3yrnJ0Qi2wa3w0/edit#heading=h.lofdeze1a2hr

Note: This can conflict with https://codereview.chromium.org/2912723002/

BUG= 604726 
TEST=gpu_unittests --gtest_filter=Buffer.*
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2909003002
Cr-Commit-Position: refs/heads/master@{#475837}

[modify] https://crrev.com/394636a1dbb10af27400f21b0326daedb3b6fede/gpu/BUILD.gn
[modify] https://crrev.com/394636a1dbb10af27400f21b0326daedb3b6fede/gpu/command_buffer/common/buffer.cc
[modify] https://crrev.com/394636a1dbb10af27400f21b0326daedb3b6fede/gpu/command_buffer/common/buffer.h
[add] https://crrev.com/394636a1dbb10af27400f21b0326daedb3b6fede/gpu/command_buffer/common/buffer_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9fb909c390cc47adc70f9592565474acc5f875d9

commit 9fb909c390cc47adc70f9592565474acc5f875d9
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Thu Jun 01 03:42:12 2017

Add TransferBufferInterface::shared_memory_handle()

This CL adds a base::SharedMemoryHandle getter to
gpu::TransferBufferInterface. The getter will be used by
TransferBufferInterface users when OnMemoryDump is called to create
base::SharedMemory dumps.

This is part of work for exposing SharedMemory usages on tracing
memory-infra dumps.

Design doc: https://docs.google.com/document/d/16Mi5_puxKgQ-9IX7ANfbji0WiqBCK3yrnJ0Qi2wa3w0/edit#heading=h.lofdeze1a2hr

BUG= 604726 
TEST=gpu_unittests --gtest_filter=TransferBufferTest.*
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2910093004
Cr-Commit-Position: refs/heads/master@{#476175}

[modify] https://crrev.com/9fb909c390cc47adc70f9592565474acc5f875d9/gpu/command_buffer/client/gles2_implementation_unittest.cc
[modify] https://crrev.com/9fb909c390cc47adc70f9592565474acc5f875d9/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/9fb909c390cc47adc70f9592565474acc5f875d9/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/9fb909c390cc47adc70f9592565474acc5f875d9/gpu/command_buffer/client/transfer_buffer_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e863242981adb61aa416fda0607470df3ca0eb6

commit 3e863242981adb61aa416fda0607470df3ca0eb6
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Tue Jun 06 04:47:03 2017

Revert "Bug fix: TranferBufferManager's SharedMemory memory usage reporting was wrong"

This reverts commit 6c8897721e2df9a0ec3677d0a13d677c457e5e1e.

The previous change removes 'shared' global dumps when the backend is
not base::SharedMemory, but actually the 'shared' global dumps are
needed regardless of the backend. 'Shared' in this context means not only
base::SharedMemory but also wider things.

BUG= 604726 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2912723002
Cr-Commit-Position: refs/heads/master@{#477197}

[modify] https://crrev.com/3e863242981adb61aa416fda0607470df3ca0eb6/gpu/command_buffer/common/buffer.cc
[modify] https://crrev.com/3e863242981adb61aa416fda0607470df3ca0eb6/gpu/command_buffer/common/buffer.h
[modify] https://crrev.com/3e863242981adb61aa416fda0607470df3ca0eb6/gpu/command_buffer/service/transfer_buffer_manager.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd9170d74a1072243071753b7fa078e6794e534c

commit dd9170d74a1072243071753b7fa078e6794e534c
Author: ssid <ssid@chromium.org>
Date: Wed Jun 07 05:17:40 2017

[memory-infra] Add method to override importance of ownership edges

Introduce ProcessMemoryDump::AddOverridableOwnershipEdge which adds
edges that can be overriden by another edge by called AddOwnershipEdge
with a different importance.

BUG= 604726 

Review-Url: https://codereview.chromium.org/2911263003
Cr-Commit-Position: refs/heads/master@{#477554}

[modify] https://crrev.com/dd9170d74a1072243071753b7fa078e6794e534c/base/trace_event/memory_allocator_dump_guid.h
[modify] https://crrev.com/dd9170d74a1072243071753b7fa078e6794e534c/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/dd9170d74a1072243071753b7fa078e6794e534c/base/trace_event/process_memory_dump.h
[modify] https://crrev.com/dd9170d74a1072243071753b7fa078e6794e534c/base/trace_event/process_memory_dump_unittest.cc
[modify] https://crrev.com/dd9170d74a1072243071753b7fa078e6794e534c/third_party/WebKit/Source/platform/instrumentation/tracing/web_process_memory_dump_test.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0

commit f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0
Author: hajimehoshi <hajimehoshi@chromium.org>
Date: Wed Jun 07 18:09:18 2017

Add cc::SharedBitmap::shared_memory_handle()

This CL adds a base::SharedMemoryHandle getter to cc::SharedBitmap. The
getter will be used by SharedBitmap users when OnMemoryDump is called to
create base::SharedMemory dumps.

This is part of work for exposing SharedMemory usages on tracing
memory-infra dumps.

Design doc: https://docs.google.com/document/d/16Mi5_puxKgQ-9IX7ANfbji0WiqBCK3yrnJ0Qi2wa3w0/edit#heading=h.lofdeze1a2hr

BUG= 604726 
TEST=components_unittests --gtest_filter=HostSharedBitmapManagerTest.SharedMemoryHandle
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2909873002
Cr-Commit-Position: refs/heads/master@{#477704}

[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/cc/resources/shared_bitmap.cc
[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/cc/resources/shared_bitmap.h
[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/cc/test/test_shared_bitmap_manager.cc
[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/components/viz/display_compositor/host_shared_bitmap_manager.cc
[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/components/viz/display_compositor/host_shared_bitmap_manager_unittest.cc
[modify] https://crrev.com/f4c8be6f9795e33e6f7478dd4ef9d7755d4e14d0/services/ui/public/cpp/bitmap/child_shared_bitmap_manager.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99042d060281b6a128b910b5389f01f00ec3b891

commit 99042d060281b6a128b910b5389f01f00ec3b891
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jun 08 02:05:49 2017

Add SharedMemoryTracker::Get(Global)GUIDForTracing

This CL adds SharedMemoryTracker::GetGUIDForTracing and
GetGlobalGUIDForTracing to get GUID for memory allocation dump of
base::SharedMemory.

These will be used from base::SharedMemory users to create ownership
edges.

This is part of work for exposing SharedMemory usages on tracing
memory-infra dumps.

Design doc: https://docs.google.com/document/d/16Mi5_puxKgQ-9IX7ANfbji0WiqBCK3yrnJ0Qi2wa3w0/edit#heading=h.lofdeze1a2hr

Bug:  604726 
Change-Id: Ia409201119fe9bdf57b9c649a1241ee812c16a0d
Reviewed-on: https://chromium-review.googlesource.com/520546
Reviewed-by: siddhartha sivakumar <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: siddhartha sivakumar <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477861}
[modify] https://crrev.com/99042d060281b6a128b910b5389f01f00ec3b891/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/99042d060281b6a128b910b5389f01f00ec3b891/base/memory/shared_memory_tracker.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93876304f62aa5f7c2354c739e5ac84e61d3ea13

commit 93876304f62aa5f7c2354c739e5ac84e61d3ea13
Author: ssid <ssid@chromium.org>
Date: Fri Jun 09 03:42:05 2017

[memory-infra] Add API to ProcessMemoryDump to create ownership edges for base::SharedMemory

This cl is adapted from hajimehoshi's
chromium-review.googlesource.com/c/523684

This CL adds a new method to ProcessMemoryDump to create proper
ownership edges for base::SharedMemory. It handles both the cases
where the global dumps are created by clients and global dumps are
created by SharedMemoryTracker.

BUG= 604726 

Review-Url: https://codereview.chromium.org/2923123004
Cr-Commit-Position: refs/heads/master@{#478187}

[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/trace_event/memory_allocator_dump_guid.cc
[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/trace_event/memory_allocator_dump_guid.h
[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/trace_event/process_memory_dump.h
[modify] https://crrev.com/93876304f62aa5f7c2354c739e5ac84e61d3ea13/base/trace_event/process_memory_dump_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f

commit 35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Tue Jun 13 03:10:40 2017

Switch ownership edges of global shared dumps for base::SharedMemory

This CL changes the current existing global-shared dumps and ownership
edges with new ones by calling ProcessMemoryDump::CreateSharedMemoryOwnershipEdge.
As a result, tracing memory-infra dumps can show base::SharedMemory
usages correctly.

Note that the current CreateSharedMemoryOwnershipEdge implementation still
uses the existing dumps and edges by default until Mojo's fix to pass
SharedMemory IDs across processes is done (crbug/713763). Now this feature
can be switched by changing ProcessMemoryDump::g_use_shared_memory_guid_for_testing.

This is part of work for exposing SharedMemory usages on tracing
memory-infra dumps.

Design doc: https://docs.google.com/document/d/16Mi5_puxKgQ-9IX7ANfbji0WiqBCK3yrnJ0Qi2wa3w0/edit#heading=h.lofdeze1a2hr

Bug:  604726 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8ec1e3df244305cfd0cac76f9037cb622251fab1
Reviewed-on: https://chromium-review.googlesource.com/526816
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Reviewed-by: siddhartha sivakumar <ssid@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478877}
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/cc/raster/staging_buffer_pool.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/cc/resources/resource_provider.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/components/viz/display_compositor/host_shared_bitmap_manager.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/client/mapped_memory.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/client/mapped_memory.h
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/gpu/command_buffer/service/transfer_buffer_manager.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/35e4fd70b755009eb78195ed9dfe0e8a3a21ba0f/ui/gl/gl_image_shared_memory.cc

I found some bugs when setting g_use_shared_memory_guid true. One is that shared memory guid is not set correctly and https://chromium-review.googlesource.com/c/535433/ will fix. The other is wrong ownership edges seen at chrome://tracing memory-infra:

While importing:
Error: undefined contains a cycle
    at GlobalMemoryDump.visit (chrome://tracing/tracing.js:2488:1083)
    at GlobalMemoryDump.<anonymous> (chrome://tracing/tracing.js:2489:72)
    at Array.forEach (<anonymous>)
    at GlobalMemoryDump.visit (chrome://tracing/tracing.js:2489:34)
    at Array.forEach (<anonymous>)
    at GlobalMemoryDump.iterateRootAllocatorDumps (chrome://tracing/tracing.js:2442:271)
    at GlobalMemoryDump.<anonymous> (chrome://tracing/tracing.js:2488:848)
    at GlobalMemoryDump.iterateContainerDumps (chrome://tracing/tracing.js:2488:640)
    at GlobalMemoryDump.iterateAllRootAllocatorDumps (chrome://tracing/tracing.js:2488:788)
    at GlobalMemoryDump.traverseAllocatorDumpsInDepthFirstPostOrder (chrome://tracing/tracing.js:2490:6)

Hmm, it looks like there are cyclic edges. I'll look into this.

https://chromium-review.googlesource.com/c/535378/ for cyclic edges

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4920d627d7f5db149d76d04950ec4d22929d586

commit c4920d627d7f5db149d76d04950ec4d22929d586
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jun 15 03:39:28 2017

Update BrowserGpuMemoryBufferManager::BufferInfo's shared_memory_guid member when necessary

This is an unfinished task in crrev/2915553002. BufferInfo's
shared_memory_guid member needs to be updated whenever GpuMemoryBuffer
is created.

Bug:  604726 
Change-Id: Id35548bdd1b04e94dc95377eb7378804a637d193
Reviewed-on: https://chromium-review.googlesource.com/535433
Reviewed-by: John Bauman <jbauman@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479593}
[modify] https://crrev.com/c4920d627d7f5db149d76d04950ec4d22929d586/content/browser/gpu/browser_gpu_memory_buffer_manager.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/950d8aba4950a60d534eeee8630e0d9f09e8ae4a

commit 950d8aba4950a60d534eeee8630e0d9f09e8ae4a
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed Jun 21 19:29:44 2017

Bug fix: GUIDs for local base::SharedMemory dumps are wrong

There are cyclic ownership edges when UserSharedMemoryBaseGUIDs()
returns true. The cause is |local_shm_guid| is generated from a string
'shared_memory/...' but this is same as global base::SharedMemory dumps'
GUIDs. In the first place, |local_shm_guid| can't be generated from a
string and instead this should be generated via MemoryAllocatorDump's
constructor. This CL fixes SharedMemoryTracker::GetDumpGUIDForTracing
to take PMD and changes the implementation.

Bug:  604726 
Change-Id: Id6bdc95436e186c933abe846e7f2afbeca300a04
Reviewed-on: https://chromium-review.googlesource.com/535378
Commit-Queue: siddhartha sivakumar <ssid@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: siddhartha sivakumar <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481276}
[modify] https://crrev.com/950d8aba4950a60d534eeee8630e0d9f09e8ae4a/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/950d8aba4950a60d534eeee8630e0d9f09e8ae4a/base/memory/shared_memory_tracker.h
[modify] https://crrev.com/950d8aba4950a60d534eeee8630e0d9f09e8ae4a/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/950d8aba4950a60d534eeee8630e0d9f09e8ae4a/base/trace_event/process_memory_dump_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de0c27b10c909960c3797ffc42159fb81d8c4bbd

commit de0c27b10c909960c3797ffc42159fb81d8c4bbd
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Tue Jun 27 08:11:24 2017

Bug fix: Avoid creating dumps when getting MemoryAllocatorDump guids in ProcessMemoryDump

SharedMemoryTracker::OnMemoryDump creates dumps with their sizes and
edges between dumps for base::SharedMemory. The problem is that same
dumps might be created at ProcessMemoryDump::
CreateSharedMemoryOwnershipEdge just to retrieve GUID, and SMT::
OnMemoryDump needs to consider the state whether dumps are already
created or not.

There are bugs in SMT::OnMemoryDump. This needs to set sizes to dumps
and edges if and only if necessary, but these are not implemented yet.
As long as PMD creates dumps, the fix would be complicated. To make
matters worse, in single process mode, dumps can be duplicated and the
fix would need to consider this condition.

This CL fixes these bugs by avoiding create dumps in ProcessMemoryDump::
CreateSharedMemoryOwnershipEdge and exposing generating GUID logic
instead. After that, PMD no longer creates dumps and SMT can assume
that if the dump already exists, this is called in single process mode.

Bug:  604726 ,  735892 
Change-Id: I34e3e2d87e238258caf692db62f2a05dd5f2b545
Reviewed-on: https://chromium-review.googlesource.com/544482
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: siddhartha sivakumar <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482572}
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/memory/shared_memory_tracker.h
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/trace_event/memory_allocator_dump.cc
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/trace_event/memory_allocator_dump.h
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/trace_event/memory_allocator_dump_unittest.cc
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/de0c27b10c909960c3797ffc42159fb81d8c4bbd/base/trace_event/process_memory_dump_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ef4160b5770c280def9169f04685e3224bc6a96

commit 1ef4160b5770c280def9169f04685e3224bc6a96
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed Jun 28 19:00:26 2017

Bug fix: cc::Resource's shared_bitmap can be null even when it has shared-bitmap ID

This can happen when

1. GPU is disabled
2. Resource is created without a shared bitmap (this can happen when software
rendering is used, and shared bitmap is set at ResourceProvider::LockForRead)
3. ResourceProvider::OnMemoryDump is called before ResourceProvider::LockForRead
is called

Now shared_bitmap is referenced at ResouceProvider::OnMemoryDump and some Windows
bots have been crashing. This CL fixes those crashes.

Bug:  735173 ,  604726 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic063f54cab613ba55810fa4bd01da6a9d1077fe0
Reviewed-on: https://chromium-review.googlesource.com/547403
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: siddhartha sivakumar <ssid@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483088}
[modify] https://crrev.com/1ef4160b5770c280def9169f04685e3224bc6a96/cc/resources/resource_provider.cc

Hmm, even after crbug/713763 is fixed, there are some SharedMemory objects without their IDs on SharedMemoryTracker::OnMemoryDump. I'm now looking into this.

The problem is, in DiscarableSharedMemroy, SharedMemoryHandle is closed without unmapping, just to save file descriptor resources.

e.g. https://cs.chromium.org/chromium/src/components/discardable_memory/service/discardable_shared_memory_manager.cc?type=cs&sq=package:chromium&l=481

After closing, shared memory handle doesn't have its ID and SharedMemoryTracker is confused.

In the current implementation, SharedMemory::Close clears its handle. My current idea to fix this problem is 

1. Create a new SharedMemoryHandle constructor to accept only an ID
2. Replace the empty SharedMemoryHandle constructor with the new one in SharedMemoryClose

After this fix, SharedMemory will have a handle that doesn't have a file descriptor and size info, but an ID.

Would this work? I'm worried about this might break some assumption. Thanks.

@hajimehoshi, c#75: That sounds fine, but instead of making an "empty" SharedMemoryHandle, why don't we just give SharedMemory a GUID?

e.g. when we open a SharedMemory region from a SharedMemoryHandle, also save the GUID. :)

Hmm, I'm afraid I disagree with your idea because

1. Let's assume SharedMemory::GetGUID() would be introduced. Should we need to replace all of SharedMemoryHandle::GetGUID() with this? Of course we know SharedMemory::GetGUID() is required only in DiscardableSharedMemory instead of SharedMemoryHandle::GetGUID(), but this sounds like exposing of implementation details.

2. Simply, SharedMemory and SharedMemoryHandle would have duplicated id information, which sounds not good.

What do you think?

As ssid@ created  crbug.com/740781 , let's continue to discuss this there.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d64433a4a096a86544cd6d80f0a18ef066396ce

commit 5d64433a4a096a86544cd6d80f0a18ef066396ce
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jul 13 14:50:22 2017

Add SharedMemory::mapped_id()

When SharedMemory::Close is called, SharedMemoryHandle is cleared and
losts its ID. This means SharedMemory(Handle)'s ID is not available
after closing.

In DiscardableSharedMemory, shared memory is closed without unmmap-ing
just to save file descriptor resources, and this makes
SharedMemoryTracker confused since SharedmemoryTracker tracks mmap-ed
(and not unmmap-ed) shared memory regions, and tries to get IDs of
those regions. The problem is that sometimes SharedMemoryTarcker fails
to get such IDs.

This CL fixes this problem by adding SharedMemory::mapped_id() to make
the id available even after Close is called.

Bug:  604726 ,  740781 
Change-Id: I5dbf6bd133d8a1e935adad53604f3f40da3c4a6f
Reviewed-on: https://chromium-review.googlesource.com/566353
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486375}
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory.h
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_fuchsia.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_mac.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_nacl.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_posix.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_unittest.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_win.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9ad7246efc2ebda81597b3c0212208c5705325a

commit d9ad7246efc2ebda81597b3c0212208c5705325a
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jul 20 04:14:40 2017

Bug fix: SharedMemory's mapped IDs can be empty on OnMemoryDump

On OnMemoryDump, base::SharedMemory's IDs are used to create ownership
edges. However, there is no guarantee that the ID is still valid when
OnMemoryDump is called. Actually, some crashes [1] are found when
MemoryAllocatorDumpGuid::UseSharedMemoryBasedGUIDs() is true [2].

This CL fixes this bug to avoid dumping when shared memory id is
invalid (empty). Also, this CL changes OnMemoryDump use SharedMemory::
mapped_id(), that returns an valid ID only when the shared memory is
actually mapped, so that we can avoid unnecessary dumps.

[1] https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/501311
[2] This is now always false, but we plan to make this return true to
    use new ownership edges.

Bug:  604726 
Change-Id: I2400ba334860e57b9b83a785cd69f4a32b8ef151
Reviewed-on: https://chromium-review.googlesource.com/571386
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488134}
[modify] https://crrev.com/d9ad7246efc2ebda81597b3c0212208c5705325a/components/viz/service/display_embedder/server_shared_bitmap_manager.cc
[modify] https://crrev.com/d9ad7246efc2ebda81597b3c0212208c5705325a/ui/gl/gl_image_shared_memory.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52448951331fb305cd6342b4fc84542dc4d60d62

commit 52448951331fb305cd6342b4fc84542dc4d60d62
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jul 20 09:30:57 2017

Enable new ownership edges for base::SharedMemory

Now mojo is fixed to pass unique IDs across processes, Let's enable new
ownership edges for base::SharedMemory and remove old ownership edges.

Perf sheriffs: If you see sharp bumps in metrics like cc/gpu/discardable/
shared_memory this is very likely the culrprit, please revert it.

Bug:  604726 
Change-Id: Iad7bf261f3e2c884d60e935cb2635923de46b2e2
Reviewed-on: https://chromium-review.googlesource.com/557741
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488174}
[modify] https://crrev.com/52448951331fb305cd6342b4fc84542dc4d60d62/base/trace_event/memory_allocator_dump_guid.cc
[modify] https://crrev.com/52448951331fb305cd6342b4fc84542dc4d60d62/base/trace_event/memory_allocator_dump_guid.h
[modify] https://crrev.com/52448951331fb305cd6342b4fc84542dc4d60d62/base/trace_event/process_memory_dump_unittest.cc

I think this is finished!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd605e7e22a8d62582509fb8aac443330c8126c3

commit bd605e7e22a8d62582509fb8aac443330c8126c3
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Aug 03 12:04:24 2017

Remove MemoryAllocatorDumpGuid::UseSharedMemoryBasedGUIDs

As we confirmed the new oenership edges works correctly, let's remove
the logic to switch back to the old ownership edges

Bug:  604726 
Change-Id: I16845d41a5aecd0d2e565157e88cf67eb9322d15
Reviewed-on: https://chromium-review.googlesource.com/593667
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491705}
[modify] https://crrev.com/bd605e7e22a8d62582509fb8aac443330c8126c3/base/trace_event/memory_allocator_dump_guid.cc
[modify] https://crrev.com/bd605e7e22a8d62582509fb8aac443330c8126c3/base/trace_event/memory_allocator_dump_guid.h
[modify] https://crrev.com/bd605e7e22a8d62582509fb8aac443330c8126c3/base/trace_event/process_memory_dump.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/841a2089c6241c63fdf92c0bb96593cff34d6b8b

commit 841a2089c6241c63fdf92c0bb96593cff34d6b8b
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed Aug 16 10:18:47 2017

Refactoring: Remove an unused argument from CreateSharedMemoryOwnershipEdge

Now we have switched the CreateSharedMemoryOwnershipEdge implementation
not to use |client_global_dump_guid|, let's remove this.

Bug:  604726 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I11114760c0c9e4c1b97a071d9ca33bb337a40d6d
Reviewed-on: https://chromium-review.googlesource.com/608732
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494743}
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/base/trace_event/process_memory_dump.h
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/base/trace_event/process_memory_dump_unittest.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/cc/raster/staging_buffer_pool.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/cc/resources/resource_provider.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/components/discardable_memory/common/discardable_shared_memory_heap.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/components/discardable_memory/common/discardable_shared_memory_heap.h
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/components/discardable_memory/service/discardable_shared_memory_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/components/viz/host/server_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/components/viz/service/display_embedder/server_shared_bitmap_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/command_buffer/client/mapped_memory.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/command_buffer/service/transfer_buffer_manager.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/gpu/ipc/client/gpu_memory_buffer_impl_shared_memory.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/ui/gfx/BUILD.gn
[delete] https://crrev.com/7a3aff75d259d51c76cb3fe2136b84dd1fec25d3/ui/gfx/gpu_memory_buffer_tracing.cc
[delete] https://crrev.com/7a3aff75d259d51c76cb3fe2136b84dd1fec25d3/ui/gfx/gpu_memory_buffer_tracing.h
[modify] https://crrev.com/841a2089c6241c63fdf92c0bb96593cff34d6b8b/ui/gl/gl_image_shared_memory.cc