Issue metadata
Sign in to add a comment
|
RequestGlobalDump should always request SharedMemory MDP on macOS |
||||||||||||||||||||||
Issue description
Current header says:
"""
// Requests a global memory dump with |allocator_dump_names| indicating
// the name of allocator dumps in which the consumer is interested. If
// |allocator_dump_names| is empty, no dumps will be returned.
// Returns asynchronously, via the callback argument:
// (true, global_dump) if succeeded;
// (false, global_dump) if failed, with global_dump being non-null
// but missing data.
// The callback (if not null), will be posted on the same thread of the
// RequestGlobalDump() call.
void RequestGlobalDump(const std::vector<std::string>& allocator_dump_names,
RequestGlobalDumpCallback);
"""
On macOS, SharedMemory MDP is required for computation of private memory footprint. But maybe we shouldn't return the MDP in the dump returned to the caller?
,
Mar 8 2018
Oh, maybe I'm mis-interpreting the API then. I thought the point was to avoid doing a bunch of extraneous work [e.g. querying MDPs and then ignoring their results]
,
Mar 8 2018
Ahhh no that was not the motivation to pass in these names. The motivation was actually to avoid large IPC messages over Mojo by returning stuff which the caller was not interested in.
,
Mar 8 2018
+ primiano, ssid Would it make sense to avoid querying MDPs that are not being requested by the caller of RequestGlobalDump? This seems relevant for https://bugs.chromium.org/p/chromium/issues/detail?id=812426. Ideally, something requesting a GlobalDump to get PMFs should not get blocked by a stalled sync thread.
,
Mar 8 2018
Ideally yes we would indeed avoid querying MDPs but we do not know what keys that a certain MDP would put into a PMD without actually invoking it first.
,
Mar 8 2018
Should we instead change the API to take MDP_names rather than allocator_dump_names then? It looks like the only caller is in https://cs.chromium.org/chromium/src/chrome/browser/metrics/process_memory_metrics_emitter.cc?type=cs&q=%22RequestGlobalDump(%22+-file:out/debug&sq=package:chromium&l=312, so it's not too late to make a change to the API itself.
,
Mar 9 2018
The alternative would be to make another API that returns just {Private/Shared/Total}MemoryFootprint, as there are several callers than need just that within Chrome. [e.g. TAsk Manager, OOM details, feedback: https://chromium-review.googlesource.com/c/chromium/src/+/951754/9/chrome/browser/memory_details.cc]
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by lalitm@chromium.org
, Mar 8 2018