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

Issue 706547 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug
Hotlist-MemoryInfra



Sign in to add a comment

Memory-mapped file not showing up in memory-infra

Project Member Reported by asvitk...@chromium.org, Mar 29 2017

Issue description

Memory-mapped file not showing up in memory-infra.

I was trying to use memory-infra as a sanity check to see memory impact of PersistentHistograms feature - which uses a memory-mapped file as backing store for all histogram data.

The persistent histograms code when running in mode "MappedFile" (which is default on TOT and currently 50%/50% on canary/dev/beta - enabled group having name "EnabledOnDiskNoStability3") will create and memory-map a file that will be used internally by histograms subsystem for all histogram data.

Relevant code:
https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_field_trials.cc?rcl=f8e03c0c5adc572d5e1f51e4093cecb0e661f05e&l=94
https://cs.chromium.org/chromium/src/base/metrics/persistent_histogram_allocator.cc?rcl=f8e03c0c5adc572d5e1f51e4093cecb0e661f05e&l=731

When I ran Canary that had the right experiment group, I could verify that the ~/Library/Application Support/Google/Chrome Canary/BrowserMetrics-active.pma file was open according to Activity Monitor on Mac.

However, it did not show up in memory-infra (following go/chrome-memory-debugging-steps steps) in the breakdown. I would have expected it to be shown under "Other" category where various files are shown - but it was not there.
 
And specifically this was for browser process. (Renderers are also affected by PersistentHistograms trial, but the BrowserMetrics-active.pma is only used by browser process.)
Each sub-process has an independent shared memory segment between it and the browser.  These aren't backed by disk files but are still mapped.
Status: Started (was: Assigned)
This is a bug in region iteration logic:
https://codereview.chromium.org/2786733004/

I've confirmed that the file in question shows up without this bug.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2017

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

commit f955ebf64b34ec9cbdcb137c00bc9bba1867d071
Author: erikchen <erikchen@chromium.org>
Date: Wed Mar 29 23:59:01 2017

macOS: Fix a bug in logic to get all memory regions.

The logic was accidentally double-counting the size of a memory region, causing
some memory regions to get skipped.

BUG= 706547 

Review-Url: https://codereview.chromium.org/2786733004
Cr-Commit-Position: refs/heads/master@{#460585}

[modify] https://crrev.com/f955ebf64b34ec9cbdcb137c00bc9bba1867d071/components/tracing/common/process_metrics_memory_dump_provider.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2017

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

commit 6387395d1a02579a33652086a64384311d576ba0
Author: stgao <stgao@chromium.org>
Date: Thu Mar 30 01:50:50 2017

Revert of macOS: Fix a bug in logic to get all memory regions. (patchset #1 id:1 of https://codereview.chromium.org/2786733004/ )

Reason for revert:
This failed ProcessMetricsMemoryDumpProviderTest.NoDuplicateRegions on "Mac ASan 64 Tests (1)".

https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/28499

Original issue's description:
> macOS: Fix a bug in logic to get all memory regions.
>
> The logic was accidentally double-counting the size of a memory region, causing
> some memory regions to get skipped.
>
> BUG= 706547 
>
> Review-Url: https://codereview.chromium.org/2786733004
> Cr-Commit-Position: refs/heads/master@{#460585}
> Committed: https://chromium.googlesource.com/chromium/src/+/f955ebf64b34ec9cbdcb137c00bc9bba1867d071

TBR=primiano@chromium.org,erikchen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 706547 

Review-Url: https://codereview.chromium.org/2788453002
Cr-Commit-Position: refs/heads/master@{#460619}

[modify] https://crrev.com/6387395d1a02579a33652086a64384311d576ba0/components/tracing/common/process_metrics_memory_dump_provider.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2017

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

commit 26bd6d86d59d2c7227e4359d91852426c80134e2
Author: erikchen <erikchen@chromium.org>
Date: Fri Mar 31 00:10:52 2017

[Reland #1] macOS: Fix a bug in logic to get all memory regions.

This CL removes the test NoDuplicateRegions, since grabbing a snapshot of all
regions is not an atomic operation - the very act of iterating through all
regions and recording stats can change the set of all regions.

> macOS: Fix a bug in logic to get all memory regions.
>
> The logic was accidentally double-counting the size of a memory region, causing
> some memory regions to get skipped.
>
> BUG= 706547 
>
> Review-Url: https://codereview.chromium.org/2786733004
> Cr-Commit-Position: refs/heads/master@{#460585}
> Committed: https://chromium.googlesource.com/chromium/src/+/f955ebf64b34ec9cbdcb137c00bc9bba1867d071

BUG= 706547 
TBR=primiano@chromium.org

Review-Url: https://codereview.chromium.org/2787143002
Cr-Commit-Position: refs/heads/master@{#460944}

[modify] https://crrev.com/26bd6d86d59d2c7227e4359d91852426c80134e2/components/tracing/common/process_metrics_memory_dump_provider.cc
[modify] https://crrev.com/26bd6d86d59d2c7227e4359d91852426c80134e2/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment