Proposal to remove "total resident", "private dirty", "swapped" columns from analysis view. |
|||
Issue descriptionHi all. These columns seem out-dated now that we have private memory footprint. On macOS, total resident is slightly negative, "private dirty" is meaningless, since it primarily measures shared, resident system libraries, and "swapped" is part of "private memory footprint". Are these columns more useful on other platforms? Unless they actually help developers solve problems, I'd like to propose getting rid of them, since more columns = more confusion.
,
Feb 27 2018
Great. I'd also like to remove "peak total resident", since we're removing "total resident". ssid@, dskiba@, is that okay?
,
Mar 1 2018
I would like to have the private_dirty and swapped in the Overview section. It is easier to see the breakdown rather than just private footprint. Sometimes we work on reducing swap rather than private footprint since that is what is slowing us down. I agree we should remove total resident from Overview section. The sum does not make any sense. Maybe a good idea to move private footprint as the first column. I remember we had different computation for private footprint, one was quick and one was accurate. Does this happen differently on light and heavy dumps? if so we need to show a popup saying how the metric was computed in the UI. I would still prefer to have all the metrics shown in the Analysis section. It is sometimes critical to look at all the numbers to understand what is going on on linux and android. Since we show all the memory mapped regions, it is better to show "total resident" as the sum of resident size of all the mapped regions. Side note: If you say some metrics are useless, or not helpful on Mac, we should just not record them and the UI should show only metrics that were recorded.
,
Mar 1 2018
Re comment#2: Yes I don't ever remember using the peak resident metric. But tbh we never tried to work on reducing peak memory usage. Maybe it might be useful at some time in future. I am ok with removing it. There seems to be no easy way to show it hidden. By removing total resident, the metric will not be seen in the overview section in both detailed and background traces. But, it can still be seen as the total of resident size of mapped regions in analysis section. I am good with that.
,
Mar 1 2018
Thanks for the feedback, here's the plan: 1) Stop emitting "total resident" from Chrome. Note that on Linux, the information is still available in the analysis view as the sum of "Private Dirty", "Private Clean", "Shared Dirty", "Shared Clean". And for more users, PMF is a much more useful number. https://chromium-review.googlesource.com/c/chromium/src/+/940144 2) Stop emitting "PSS", "Private Dirty", "Private Clean", "Shared Dirty", "Shared Clean" on macOS and Windows. There are no platform-equivalents. The numbers were already not being collected on Windows. Stop attempting to collect them on maCOS. https://chromium-review.googlesource.com/c/chromium/src/+/943184
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3032c633775bcde85fdef8a0136a98a5320fd7e9 commit 3032c633775bcde85fdef8a0136a98a5320fd7e9 Author: erikchen <erikchen@chromium.org> Date: Thu Mar 01 17:26:05 2018 Stop emitting "resident_set_bytes" in memory-infra traces. Now that we emit private memory footprint, the former number is not particularly meaningful. Bug: 815328 Change-Id: I85cb76b08ef85073179b5c334c16d4775667b8e6 Reviewed-on: https://chromium-review.googlesource.com/940144 Commit-Queue: Erik Chen <erikchen@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org> Cr-Commit-Position: refs/heads/master@{#540197} [modify] https://crrev.com/3032c633775bcde85fdef8a0136a98a5320fd7e9/services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.cc
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/406b235a95c475c4bd67cf13f46603a5a2e027e8 commit 406b235a95c475c4bd67cf13f46603a5a2e027e8 Author: erikchen <erikchen@chromium.org> Date: Fri Mar 02 13:21:17 2018 Fix logic in memory dump event. Byte stats are Linux specific, and will not be present on macOS and Windows. Fix the logic in memory dump event to not throw an exception if the field is not present. Bug: chromium:815328 Change-Id: If12bf94577fc97da2fee45428e0bb5fa099c87f8 Reviewed-on: https://chromium-review.googlesource.com/944004 Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> [modify] https://crrev.com/406b235a95c475c4bd67cf13f46603a5a2e027e8/telemetry/telemetry/timeline/memory_dump_event.py
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1b73669d1e45b07a2d7a4cf0cfd5dbfe08d2a6d commit a1b73669d1e45b07a2d7a4cf0cfd5dbfe08d2a6d Author: Erik Chen <erikchen@chromium.org> Date: Fri Mar 02 17:23:35 2018 Stop emitting Linux-specific VMRegion stats on non-Linux derived OSes. The stats are not meaningful. They were already not being collected on Windows. Stop attempting to collect macOS stats [which have different semantics]. Bug: 815328 Change-Id: Ic25eb3c16be47aa87dfb2769fc97a2079b418d34 Reviewed-on: https://chromium-review.googlesource.com/943184 Commit-Queue: Erik Chen <erikchen@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org> Cr-Commit-Position: refs/heads/master@{#540544} [modify] https://crrev.com/a1b73669d1e45b07a2d7a4cf0cfd5dbfe08d2a6d/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_mac.cc [modify] https://crrev.com/a1b73669d1e45b07a2d7a4cf0cfd5dbfe08d2a6d/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_unittest.cc [modify] https://crrev.com/a1b73669d1e45b07a2d7a4cf0cfd5dbfe08d2a6d/services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.cc
,
May 21 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by primiano@chromium.org
, Feb 26 2018