New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 854097 link

Starred by 2 users
Status: Fixed
Owner: ----
Closed: Dec 4
Cc:
█ dinfuehr@google.com
alph@chromium.org
u...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Show links to the source in DevTools retainers view

Project Member Reported by u...@chromium.org, Jun 19 2018 
For some of the retainers in DevTools heap snapshot we can infer the corresponding source position. Showing a link to the source would simplify debugging.

Retainers that can be linked:
- variables: link to the "let, const, var" statement.
- classes: link to the class definition or the constructor function.
- closures: link to the function definition.

Comment 1 by u...@chromium.org, Jul 6

Cc: dinfuehr@google.com
Status: Assigned (was: Available)
dinfuehr@ will work on this.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 24 

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

commit 78d611582e176735c229965589cbb837bf08dc2a
Author: Dominik Inführ <dinfuehr@google.com>
Date: Tue Jul 24 13:55:55 2018

DevTools: no link when snapshot loaded from file

Code failed when snapshot was not taken from the running application but
loaded from file. heapProfilerModel() returns null if snapshot was loaded
from file.

Bug:  chromium:854097 
Change-Id: I3c269a78f7f8ec59a15f510dda6ab55a72f53616
Reviewed-on: https://chromium-review.googlesource.com/1146921
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577515}
[modify] https://crrev.com/78d611582e176735c229965589cbb837bf08dc2a/third_party/blink/renderer/devtools/front_end/profiler/HeapSnapshotView.js
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 24 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/956ac1bd1574d4664b6796d69625a0a6a28a8c9e

commit 956ac1bd1574d4664b6796d69625a0a6a28a8c9e
Author: Dominik Inführ <dinfuehr@google.com>
Date: Tue Jul 24 14:49:03 2018

[heap-profiler] Names for JSGeneratorObject-fields

Add names for fields in JSGeneratorObjects in Heap Snapshot
Generator.

Bug:  chromium:854097 
Change-Id: I075acf0821c9d002535b4fdc4ce4ddbb2fc9627c
Reviewed-on: https://chromium-review.googlesource.com/1148387
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54652}
[modify] https://crrev.com/956ac1bd1574d4664b6796d69625a0a6a28a8c9e/src/objects.h
[modify] https://crrev.com/956ac1bd1574d4664b6796d69625a0a6a28a8c9e/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/956ac1bd1574d4664b6796d69625a0a6a28a8c9e/src/profiler/heap-snapshot-generator.h
[modify] https://crrev.com/956ac1bd1574d4664b6796d69625a0a6a28a8c9e/test/cctest/test-heap-profiler.cc

Comment 4 by jo...@igalia.com, Jul 27 

Since https://bugs.chromium.org/p/v8/issues/detail?id=7066 got fixed there doesn't seem to be a way to obtain the information about the Script (filenames, .etc) from a context. Will the new design restore the association in HeapSnapshots somehow? This would be very useful for downstream tools.

Comment 5 by dinfuehr@google.com, Aug 2 

Yes, currently the script needs to be determined from the closure that keeps the context alive. Even with this solution there might be some issues with eval.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 21 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/64e04c96af6a7b001ab747180bb440e6962d04b8

commit 64e04c96af6a7b001ab747180bb440e6962d04b8
Author: Dominik Inführ <dinfuehr@google.com>
Date: Tue Aug 21 08:23:00 2018

[heap-profiler] Store locations in snapshot

Start storing locations in heap snapshot file. Initial support
for closure, additional object types might be added in the future.
Needed to show source code locations for objects in the DevTools
heap snapshot viewer.

Bug:  chromium:854097 
Change-Id: I12659373ce1adf67b55c6a10ea1d0465fcdb4a10
Reviewed-on: https://chromium-review.googlesource.com/1174257
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55245}
[modify] https://crrev.com/64e04c96af6a7b001ab747180bb440e6962d04b8/src/profiler/heap-snapshot-generator-inl.h
[modify] https://crrev.com/64e04c96af6a7b001ab747180bb440e6962d04b8/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/64e04c96af6a7b001ab747180bb440e6962d04b8/src/profiler/heap-snapshot-generator.h
[modify] https://crrev.com/64e04c96af6a7b001ab747180bb440e6962d04b8/test/cctest/test-heap-profiler.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 21 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/32ec3c1c5ed363384f41fd469b5a233b795038c1

commit 32ec3c1c5ed363384f41fd469b5a233b795038c1
Author: Dominik Inführ <dinfuehr@google.com>
Date: Tue Aug 21 10:32:56 2018

[heap-profiler] Generate location for generators

Add source code location for generators into heap snapshot file.

Bug:  chromium:854097 
Change-Id: I726b245a707515502976476703e57b7f58c92782
Reviewed-on: https://chromium-review.googlesource.com/1174433
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55254}
[modify] https://crrev.com/32ec3c1c5ed363384f41fd469b5a233b795038c1/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/32ec3c1c5ed363384f41fd469b5a233b795038c1/src/profiler/heap-snapshot-generator.h
[modify] https://crrev.com/32ec3c1c5ed363384f41fd469b5a233b795038c1/test/cctest/test-heap-profiler.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 24 

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

commit ff7434107cc00bc0752dd0f0930bcf4985e7f443
Author: Dominik Inführ <dinfuehr@google.com>
Date: Fri Aug 24 10:01:48 2018

[heap-profiler] Location for object's constructor

Add location information in heap snapshot for objects where the
constructor can be determined.

Bug:  chromium:854097 
Change-Id: Ieb2ab70a65809ecc9dfa0d73a33fa57add430465
Reviewed-on: https://chromium-review.googlesource.com/1179156
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55387}
[modify] https://crrev.com/ff7434107cc00bc0752dd0f0930bcf4985e7f443/src/objects.cc
[modify] https://crrev.com/ff7434107cc00bc0752dd0f0930bcf4985e7f443/src/objects.h
[modify] https://crrev.com/ff7434107cc00bc0752dd0f0930bcf4985e7f443/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/ff7434107cc00bc0752dd0f0930bcf4985e7f443/src/profiler/heap-snapshot-generator.h
[modify] https://crrev.com/ff7434107cc00bc0752dd0f0930bcf4985e7f443/test/cctest/test-heap-profiler.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 27 

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

commit 5fb97b76ec296f803270d1fecef27bc066d724cb
Author: Dominik Inführ <dinfuehr@google.com>
Date: Mon Aug 27 16:24:00 2018

DevTools: Load location from snapshot

Load locations from the snapshot file and do not call into V8
for it. Requires V8 to store locations in the snapshot. This speeds up
location access. It should also enable us to show locations for snapshots
loaded from file in the future.

Bug:  chromium:854097 
Change-Id: Ia35978a63fc30687e5fd9a711023384d9875a9a7
Reviewed-on: https://chromium-review.googlesource.com/1174254
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586279}
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-expected.txt
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/heap_profiler_test_runner/HeapProfilerTestRunner.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/heap_snapshot_model/HeapSnapshotModel.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/heap_snapshot_worker/HeapSnapshot.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/heap_snapshot_worker/HeapSnapshotLoader.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/profiler/HeapSnapshotGridNodes.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/profiler/HeapSnapshotProxy.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/profiler/HeapSnapshotView.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/profiler/ProfileType.js
[modify] https://crrev.com/5fb97b76ec296f803270d1fecef27bc066d724cb/third_party/blink/renderer/devtools/front_end/profiler/ProfilesPanel.js
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 28 

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

commit d0c578b4169dda909d3b5aa7f7739e148f42f38e
Author: Dominik Inführ <dinfuehr@google.com>
Date: Tue Aug 28 09:47:13 2018

DevTools: Show locations for non-closures

In the DevTools heap snapshot tool show source code locations
also for non-closures. This is now cheaper since we can get this
information from the heap snapshot. Showing more source code locations
only requires adding more location information into the heap snapshot,
DevTools then can automically make use of it.

Bug:  chromium:854097 
Change-Id: I1f7444c773586df0f7b542411333b46d73b18b93
Reviewed-on: https://chromium-review.googlesource.com/1180883
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Cr-Commit-Position: refs/heads/master@{#586634}
[delete] https://crrev.com/2a9edcc33f9506417c0f4e209c349b3e8cf3eb7f/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-function-location-expected.txt
[delete] https://crrev.com/2a9edcc33f9506417c0f4e209c349b3e8cf3eb7f/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-function-location.js
[add] https://crrev.com/d0c578b4169dda909d3b5aa7f7739e148f42f38e/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-location-expected.txt
[add] https://crrev.com/d0c578b4169dda909d3b5aa7f7739e148f42f38e/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-location.js
[modify] https://crrev.com/d0c578b4169dda909d3b5aa7f7739e148f42f38e/third_party/blink/renderer/devtools/front_end/profiler/HeapSnapshotGridNodes.js
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 29 

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

commit 78867c06919268e1d1f1af9ccea9c0a96e550eaf
Author: Dominik Inführ <dinfuehr@google.com>
Date: Wed Aug 29 01:52:26 2018

DevTools: Test for object constructor location

Add test for object constructor location. V8 now generates this
information into heap snapshots, add this test to keep this working.

Bug:  chromium:854097 
Change-Id: Ia94a10212d5f40aa7589c4142b0132c180dec969
Reviewed-on: https://chromium-review.googlesource.com/1188469
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586992}
[modify] https://crrev.com/78867c06919268e1d1f1af9ccea9c0a96e550eaf/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-location-expected.txt
[modify] https://crrev.com/78867c06919268e1d1f1af9ccea9c0a96e550eaf/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-location.js

Comment 12 by pfeldman@chromium.org, Dec 4

Status: Fixed (was: Assigned)
Looks fixed
►

Sign in to add a comment