Issue metadata
Sign in to add a comment
|
1% regression in system_health.memory_mobile at 544932:544974 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 26 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/16ad39e3440000
,
Apr 1 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/16ad39e3440000 Convert Cache Storage IPC to Mojo. by lucmult@chromium.org https://chromium.googlesource.com/chromium/src/+/626c99e870b04e474408a5b35e97f260b9d787ba Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Apr 3 2018
Hi,
This is my first "memory regression" so I'm not sure where else to look.
1. The report says "malloc" for render process, with an increase of ~400KiB.
2. This 400KiB it's classified as "malloc" -> "allocated_objects" -> "<unspecified>", which I don't know what exactly means.
I'm describing below where my patch changes allocated objects and their management, so other memory benchmark specialists can help me to narrow that down.
* content/browser/cache_storage/cache_storage_dispatcher_host.[h|cc]
* CacheStorageDispatcherHost:
* migrated from BrowserMessageFilter to RefCountedThreadSafe<>
* implements Mojo interface blink:mojom::CacheStorage
* holds mojo bindings via `mojo::BindingSet<blink::mojom::CacheStorage, url::Origin> bindings_`, one per origin.
* holds binding for `mojo::StrongAssociatedBindingSet<blink::mojom::CacheStorageCache> cache_bindings_` one per "cache", which can be multiple per origin.
* it's allocated with base::MakeRefCounted<>.
* CacheImpl:
* new type, it implements Mojo interface `blink::mojom::CacheStorageCache`.
* it's allocated with std::make_unique<>.
* content/browser/renderer_host/render_process_host_impl.[h|cc]
* owns a scoped_refptr to CacheStorageDispatcherHost.
* content/renderer/cache_storage/webserviceworkercachestorage_impl.[h|cc]
* WebServiceWorkerCacheStorageImpl:
* it's the implementation of blink::WebServiceWorkerCacheStorage (blink/webkit type).
* it's one instance per render process / web worker.
* it's allocated by RendererBlinkPlatformImpl with std::make_unique<>.
* it's owned by GlobalCacheStorageImpl (third_party/WebKit/Source/modules/cachestorage/GlobalCacheStorage.cpp).
* WebCache
* it's the implementation of blink::WebServiceWorkerCache (blink/webkit type).
* allocated with std::make_unique<> (on WebServiceWorkerCacheStorageImpl) and forwarded(std::move) to caller/js.
* CacheRef
* it relates 1 to 1 with WebCache (above), it's owned by WebCache.
* it's allocated with base::MakeRefCounted<>.
----
It's unclear to me what would be reported as "malloc <unspecified>" out of those types above.
My initial suspicious is on the WebCache <=> CacheRef, which is new, but I can't see how it would increase memory usage.
I tried to access https://www.washingtonpost.com/pwa to try to collect a live trace or something or add some logging messages, but this URL isn't working. Is there a way to connect my locally built chrome binary to the affected site?
Is there a way to instrument the code to gather more information around those types to try narrow down this? I'm thinking on some logging or trace.
perezju@ can you help me with this analysis or with my questions on the previous 2 paragraphs?
,
Apr 3 2018
rockot@, can this ~400KiB be related to Mojo internals like on the 2 binding sets I'm using on this patch? Also, I noticed that the memory trace has a column for "mojo" but it doesn't have anything reported (before nor after). Attached a screenshot comparing the before/after memory trace, but you can also probably see it from the links below: Before: https://00e9e64baca580b34c6caa4a54b97602442854357a0f78265d-apidata.googleusercontent.com/download/storage/v1/b/chrome-telemetry-output/o/browse_news_washingtonpost_2018-03-21_23-30-46_63331.html?qk=AD5uMEsm0u7ycGSuP2YBP_LrzffrgTSRh4HpCXdFSInW93OBSghofkTb3choTsdzIKcrNOWt55XueRvucedULdG2eimcWNM9CAgtSNlMKSYNLaCdxnxVxIoH-VE4T-YG3iLdBF4neWyLN5L2nDQgo7GYRiZN2j9vYBaaQVSMAjHxJYITcc2rkhsa9VyEMZyYc0962NN4VIbIFIVk4sndbrzKSXGwwemCCkwewLLh2sb-OAB6odEUhqOjPn6eZsB-BI0WsStnwtDEwqkY_SuzHYEqhe30cH9dHOxRmmbohh-iQmw2S6sUxYpLOvzlgqI5eWpK9IpViKW7IVDYWcdFrJ026qO4qZ5INB2JYKQzg6ojd5TSVhu32I37c_nwrkofhtrnYPXZLRtGPXNJkHf0JLYmgzKS3nltCvkueCBITgFEblbn0mFXKq5vLa0I1erGP9m6mCX63PlcBy9d6Sy_0zUvQWsioX2BsAL4jDB8-vrjX_7wETXRxA8jxafX_ctVngSBWw8muZrPgTTFWwpXrWj6Ix57BoJWbXdSDDt0I9NiJu3G5HnHTZLgbdNKNHy20ZWUcFrgaNtNznhN_J8Wx2wC3cuajANUl4GjB_VujYhORp6rnxqKXFfko8L4ctpiikV9AsWRfIkscWL_Wybz7r3X2RkIPdAdQTHgWhs7ljo-VOLxcYTLzIGJ49fV7prpFzWJZRxu577neEib1ynsbscRP9KmVs64chgZBdPVDa_3sHsh_eFukeweTssUq7L23v5YhDvzQnOMB6bVFo4f0NUzkDOpVLTl53wUQWUEUeg7CDWV837zsIQ After: https://00e9e64bacd650d8609790a357b2f6f4c4e917b959d37887e0-apidata.googleusercontent.com/download/storage/v1/b/chrome-telemetry-output/o/browse_news_washingtonpost_2018-03-22_05-30-01_61577.html?qk=AD5uMEviL3R6p2IIZ549rbxow-r6leNiMfXzCQ-YuXZeNm3rwBubybVmbagHdp5Rey1zhba7pDBYNB8AZ82p5aZEkpqjmbRY9Pop5qqmFgMtNhD25NIw1BYvwMf4D6GkJ1DGr2rYF6KwXa_mZ_A83u1SSQb1IZYEVx3qUXFM-r5u2lMKjuyenVfh3sQ8oLkyHMZFsMOCoN2xfpVI-RUTrSI-Mz89VLPc5NjNqgdw68MxBa3oToMjbRlNo0bPJ_CCRcb6eUK-NAxAexJJpJpsJlnC7yBdI8cvmsaQUcFMoI2Va-GL52TsQNS4_x9ZGmKJLfcdxW-fSMLLiRf1tVTNwmlJ4rxOBPClVCDIFWZcfJgVj71_2MNciJqT0Bn_6PxOF6YVWX4PUoNssvInvGKppe4SFkRjEK9e-hEysoo_ui9y139jhFsofJYNo4d0oTSKs253eHaHZsA4_4ZETW6qmhBNjfILY8_kckXSooqa2r1RIRhfo56PAsjVOO_Filqy9vSt_A6d_v5j2I9dfDSC4CZEvwYdLDdsynND5hBtII0wwNoQInVOx_Mcc8eDUEeZCkWsR50vJt04l-44yjTpdPVE78HnH3loe95XfSxzjaDAZKr_7mJX-nlZM4uGVt0qp2x4jOLhF-Gm9i3IWGOo7k8A5k8DFS-7N4FgT27MGQeSU498xCCoyHD0yu-wiBOZb8-lfSJcMym_ug9Wr_UM8UfBGbJTdwHepttbPfg6QMRuJLP_0iiHe_199Qzg-nnZb-gBE5STgSwNthSnydNvtqZf1qtyJ4TjTcWAPiWJiytRCWP8iF8kI6I
,
Apr 3 2018
+mariakhomenko in case you can route to someone more knowledgeable about malloc details. Reducing the Pri here since, to be honest, the size of the regression is relatively small and (looking at the first link in #1) somewhat within noise range.
,
Apr 3 2018
400 kB seems much too large to be attributable to use of Mojo bindings. There isn't anywhere near that much per-interface memory overhead. If this is being measured with a very large number of messages in flight, that might explain the extra overhead. Mojo messages are on average larger allocations than legacy IPC messages.
,
Apr 5 2018
The mojo dump provider doesn't show estimated memory (hence 0MB there) because that's expensive to compute, but if you click on the actual item it should show you the number of messages / object counts as a list.
,
Apr 5 2018
Incidentally I did land this just yesterday: https://chromium-review.googlesource.com/c/chromium/src/+/992553 It seems likely to have made a significant dent in this same metric. Maybe it's fine now.
,
Apr 6 2018
Thanks mariakhomenko@ and rockot@. From the number of Mojo objects, mojo doesn't seem to be our issue here (see attachment), it shows a delta of +11 mojo objects. mariakhomenko@ from my comment #4, can you comment on what would be counted on the "malloc" column? I noticed the links I posted befre are expired, so to see those trace you can check the Pinpoint dashboard (see attachment for more info). https://pinpoint-dot-chromeperf.appspot.com/job/16ad39e3440000
,
Apr 6 2018
Anything that's allocated on native heap by Chrome can be there. If you want to see a full list of objects in malloc you can get that by running the heap profiler manually: https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/heap_profiler.md
,
Apr 17 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14b6590ec40000
,
Apr 17 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14e1064ec40000
,
Apr 17 2018
We've identified a memory leak from a crash report crbug.com/831054 . I have a CL under review to fix this: https://chromium-review.googlesource.com/c/chromium/src/+/1011467
,
Apr 17 2018
📍 Couldn't reproduce a difference. https://pinpoint-dot-chromeperf.appspot.com/job/14b6590ec40000
,
Apr 17 2018
📍 Couldn't reproduce a difference. https://pinpoint-dot-chromeperf.appspot.com/job/14e1064ec40000
,
Apr 17 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14f0fc66c40000
,
Apr 17 2018
Patch for memory leak and crash has landed: https://chromium-review.googlesource.com/c/chromium/src/+/1011467 Comments #15 and #16 was me re-running Pinpoint with that patch, it isn't 100% clear what Pinpoint is saying there as "couldn't reproduce a difference", but I take it as the issue being fixed. I'm marking this as fixed for now specially because the memory usage is back on the levels prior to my change (see attachment). Please re-open the bug if any other step is needed from me.
,
Apr 17 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/14f0fc66c40000 Convert Cache Storage IPC to Mojo. by lucmult@chromium.org https://chromium.googlesource.com/chromium/src/+/626c99e870b04e474408a5b35e97f260b9d787ba Understanding performance regressions: http://g.co/ChromePerformanceRegressions |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Mar 26 2018