Chrome Dev tools Memory Heap Profiler Improvement
Reported by
dnalaga...@linkedin.com,
Aug 16
|
||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36 Steps to reproduce the problem: 1. Download code from the Repo - https://github.com/dnalagatla/ember-2.18.2-memory-leak 2. comment `delete self.WeakMap` in `./tests/index.html` in order to repro issue. (Hack we followed this pull polyfill for debugging). 3. `npm install` 4. `ember exam --server --filter="Integration | Component | my button"` 5. Take a Heap Snapsot and Filter for `Container` class. 6. Open `Continer` in summary view and you will notice the reference shown in a WeakMap but doesn't indicate the actual location of retain. (Attachement # 1) What is the expected behavior? Expected behavior 1. Uncomment `delete self.WeakMap` in `./tests/index.html` and polyfill of WeakMap gets used. 2. Reload the test and take another heap snapshot. 3. Check `Container` reference in summary view indicates proper location where it is getting defined. (Attachement # 2) What went wrong? While debugging a memory leak issue using Heap profiler, we noticed that with native WeakMap heap profiler was not clear to pin point the retainer of the Reference. Did this work before? N/A Chrome version: 68.0.3440.106 Channel: stable OS Version: OS X 10.13.3 Flash Version:
,
Aug 17
dnalagatla@, thanks for the report! Would it be possible to upload the heap snapshot corresponding to Attachment 1? (we cannot run `nmp install` on custom packages due to corp policy).
,
Aug 17
Hi, I attached the heap snapshot for attachment 1. Thank you for looking into this ticket. Dinesh
,
Aug 21
Thanks, dnalagatla@. The actual leaking path (via the key) appears in the snapshot, but it is hard to find it because multiple paths via the WeakMap are shorter and appear at the top. I see two possible solutions: 1) Treat the WeakMap edge as weak arguing that users are interested in the path via the key most of time. 2) Add UI option that allows to select whether the WeakMap edge is weak or strong. I am in favor of option 1. alph@ wdyt?
,
Aug 21
I thought we already mark references from weak container tables as weak. We even used to have a second pass over the heap for that. But the pass was removed as the weak references are now identified with a different approach. Have we broken it?
,
Aug 21
Here is the pass removing CL: https://chromium-review.googlesource.com/c/v8/v8/+/1092370 It preserves the weak/strong edges. "table => key" edge was weak. "table => value" edge was strong. The proposal is to make "table => value" edge weak too.
,
Aug 21
if memory serves, it did work for a short period, so maybe a regression?
,
Aug 21
@ulan. Indeed. +1 to the proposal.
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ffcee2d671e3fbec495779d80357ce8a3955eeca commit ffcee2d671e3fbec495779d80357ce8a3955eeca Author: Ulan Degenbaev <ulan@chromium.org> Date: Wed Aug 29 01:58:38 2018 [heap-profiler] Make WeakMap => key value weak in heap snapshot. Bug: chromium:875041 Change-Id: I4008fa2f7d92a0f2005c7566eb2945a800a9d284 Reviewed-on: https://chromium-review.googlesource.com/1190862 Commit-Queue: Alexei Filippov <alph@chromium.org> Reviewed-by: Alexei Filippov <alph@chromium.org> Cr-Commit-Position: refs/heads/master@{#55474} [modify] https://crrev.com/ffcee2d671e3fbec495779d80357ce8a3955eeca/src/profiler/heap-snapshot-generator.cc [modify] https://crrev.com/ffcee2d671e3fbec495779d80357ce8a3955eeca/test/cctest/test-heap-profiler.cc
,
Sep 7
The fix should be in chrome canary >= 71.0.3542.0. dnalagatla@, could you please check if it works on your example?
,
Sep 19
I verified locally on my example and it works as expected.
,
Sep 19
Thanks for verification. I'll mark it as fixed. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by phanindra.mandapaka@chromium.org
, Aug 17