Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 1408403004: [tracing] Fix resident size of malloc dump provider when tcmalloc is used (Closed)

Created:
4 years, 4 months ago by ssid
Modified:
4 years, 4 months ago
CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org, vmpstr+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[tracing] Fix resident size of malloc dump provider when tcmalloc is used When tcmalloc is used the dump provider only reports the total allocated objects size as the resident size. but in reality tcmalloc uses more resident memory than the allocated objects size since it has thread cache and free list memory that are resident. It also has malloc meta data that is resident. This CL changes the resident size of malloc to include tcmalloc's page heap free list sizes, and memory used by central, transfer and thread caches. To use GetNumericProperty function from base/trace_event, new api is added in allocator_extension. It still does not include the malloc metadata bytes since it is not returned by the tcmalloc api. So this should be added in tcmalloc. BUG=546491 Committed: https://crrev.com/0943409e7b60ead1aa0b4f4ef8b4de5617b81917 Cr-Commit-Position: refs/heads/master@{#356162}

Patch Set 1 #

Patch Set 2 : Use GetStats function. #

Patch Set 3 : Clean up. #

Patch Set 4 : Back to using GetNumericProperty. #

Total comments: 4

Patch Set 5 : Adding comment and include. #

Patch Set 6 : Use allocator_extension_thunks #

Patch Set 7 : Nit. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -12 lines) Patch
M base/allocator/allocator_extension.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M base/allocator/allocator_extension.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M base/allocator/allocator_extension_thunks.h View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M base/allocator/allocator_extension_thunks.cc View 1 2 3 4 5 2 chunks +10 lines, -0 lines 0 comments Download
M base/trace_event/malloc_dump_provider.cc View 1 2 3 4 5 6 2 chunks +45 lines, -12 lines 0 comments Download
M content/app/content_main_runner.cc View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 29 (14 generated)
ssid
PTAL, Thanks.
4 years, 4 months ago (2015-10-22 16:41:45 UTC) #3
Primiano Tucci (use gerrit)
On 2015/10/22 16:41:45, ssid wrote: > PTAL, Thanks. Parsing a debug string used for himan ...
4 years, 4 months ago (2015-10-22 17:24:04 UTC) #7
ssid
On 2015/10/22 17:24:04, Primiano Tucci wrote: > On 2015/10/22 16:41:45, ssid wrote: > > PTAL, ...
4 years, 4 months ago (2015-10-22 17:26:39 UTC) #8
ssid
On 2015/10/22 17:26:39, ssid wrote: > On 2015/10/22 17:24:04, Primiano Tucci wrote: > > On ...
4 years, 4 months ago (2015-10-22 17:59:36 UTC) #9
Primiano Tucci (use gerrit)
On 2015/10/22 17:59:36, ssid wrote: > On 2015/10/22 17:26:39, ssid wrote: > > On 2015/10/22 ...
4 years, 4 months ago (2015-10-26 13:16:45 UTC) #10
ssid
Done, PTAL Thanks.
4 years, 4 months ago (2015-10-26 14:05:52 UTC) #14
Primiano Tucci (use gerrit)
LGTM with two comments https://codereview.chromium.org/1408403004/diff/140001/base/trace_event/malloc_dump_provider.cc File base/trace_event/malloc_dump_provider.cc (right): https://codereview.chromium.org/1408403004/diff/140001/base/trace_event/malloc_dump_provider.cc#newcode9 base/trace_event/malloc_dump_provider.cc:9: #include "base/trace_event/process_memory_dump.h" please add an ...
4 years, 4 months ago (2015-10-26 14:25:35 UTC) #15
ssid
Changed to add api in allocator_extension_thunks , which base can use. https://codereview.chromium.org/1408403004/diff/140001/base/trace_event/malloc_dump_provider.cc File base/trace_event/malloc_dump_provider.cc (right): ...
4 years, 4 months ago (2015-10-26 17:10:40 UTC) #17
ssid
+wfh: Please review the changes for exposing the GetNumericProperty method for tcmalloc. changes in: base/allocator ...
4 years, 4 months ago (2015-10-26 17:14:09 UTC) #20
Will Harris
lgtm for base/allocator. you'll need to find a content/ owner for the wiring up of ...
4 years, 4 months ago (2015-10-26 20:54:32 UTC) #21
Primiano Tucci (use gerrit)
+sievers for the wire-up in content/
4 years, 4 months ago (2015-10-26 21:00:20 UTC) #23
no sievers
content lgtm
4 years, 4 months ago (2015-10-26 21:41:38 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408403004/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408403004/200001
4 years, 4 months ago (2015-10-26 22:24:20 UTC) #27
commit-bot: I haz the power
Committed patchset #7 (id:200001)
4 years, 4 months ago (2015-10-26 23:05:11 UTC) #28
commit-bot: I haz the power
4 years, 4 months ago (2015-10-26 23:06:07 UTC) #29
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/0943409e7b60ead1aa0b4f4ef8b4de5617b81917
Cr-Commit-Position: refs/heads/master@{#356162}

Powered by Google App Engine
This is Rietveld 408576698