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

Issue 605785 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

MemoryInfra: Add a memory dump provider for DOM storage.

Project Member Reported by ssid@chromium.org, Apr 21 2016

Issue description

Background context:go/memory-infra: memory profiling in chrome://tracing

DOM storage uses a lot of memory on long running browser. So, it should be accounted in memory-infra.

It uses memory in 3 ways:
1. A map buffer in DomStorageArea before committing to database.
2. A session storage database backed by leveldb.
3. A local storage database backed by sqlite.

The tricky part here is only the sqlite databases are accounted in memory-infra. So, (3) has to be sub-allocated from the sqlite dumps in some way.

Also, the dom databases in (1) are accessed using DOMStorageTaskRunner. But a SequencedTaskRunner can be extracted from this worker pool just like SequencedWorkerPool::GetSequencedTaskRunner for the PRIMARY id to get memory dumps. This is because Memory-infra only supports dumps on SequencedTaskRunner.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2016

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

commit d0fd26c479bfa0b52f3764ee8925a1c82bf305ca
Author: ssid <ssid@chromium.org>
Date: Wed May 04 07:28:46 2016

[tracing] Add a memory dump provider for DOM storage

This CL adds a memory dump provider for DOM storage in browser. It only
accounts for the memory usage of the buffer map and session storage
database, not the local backing databases. This will be done in next CL.
The areas can be accessed only on DOMStorageTaskRunner's
PRIMARY_SEQUENCE. Since memory infra supports collection of dumps from
SequencedTaskRunner, a SequencedTaskRunner is extracted from
DOMStorageTaskRunner's worker pool to pass to memory-infra.

BUG= 605785 

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

[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_area.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_area.h
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_context_impl.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_context_impl.h
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_context_impl_unittest.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_namespace.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_namespace.h
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_task_runner.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/dom_storage_task_runner.h
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/session_storage_database.cc
[modify] https://crrev.com/d0fd26c479bfa0b52f3764ee8925a1c82bf305ca/content/browser/dom_storage/session_storage_database.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 8 2016

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

commit 659a76cbecfd264aa1c04c113e5d0a03b88038fc
Author: ssid <ssid@chromium.org>
Date: Wed Jun 08 07:19:15 2016

[tracing] Support background mode in dom storage memory dumps

Provide just total numbers for background mode dumps in dom storage.
The storage could be using memory for commit batches along with the
storage maps. So, add the size to tracing.

BUG= 613198 ,  605785 
TBR=michaeln

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

[modify] https://crrev.com/659a76cbecfd264aa1c04c113e5d0a03b88038fc/content/browser/dom_storage/dom_storage_area.cc
[modify] https://crrev.com/659a76cbecfd264aa1c04c113e5d0a03b88038fc/content/browser/dom_storage/dom_storage_context_impl.cc

Comment 3 by ssid@chromium.org, Jun 24 2016

Labels: -Pri-2 Pri-3
Lowering priority since the only part left is to display the sqlite usage under dom. But everything is currently being recorded.
Status: Started (was: Untriaged)
triacing: should this still be open?

Comment 5 by ssid@chromium.org, Nov 29 2016

Status: Fixed (was: Started)
Yes, I don't think it is really worth the code complexity to change sqlite reporting to dom_storage.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 9 2016

Components: Internals>Instrumentation>Memory

Sign in to add a comment