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

Issue 762639 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Enable leveldb dump providers in background mode

Project Member Reported by ssid@chromium.org, Sep 6 2017

Issue description

Background context: go/memory-infra

The new LevelDB tracker tracks all databases in Chrome. This dump provider should be enabled in background mode. This provider should not use a lot of cpu or create large number of dumps. So, should be fine to enable in background mode.

Problems:
1. The existing LocalStorageContextMojo creates a leveldb dump that does not have size. The size is added by LevelDBDatabaseImpl on the service side using a global dump. Since we need this size to be attributed to local storage, the background mode should support global dumps edges. Currently the background whitelist does not allow creation of "global" dumps.
2. The levelDB tracker must be enabled in background mode, after confirming that the performance is not affected. This can be done before fixing the local storage issue.

 

Comment 1 by ssid@chromium.org, Sep 6 2017

Lalith@ take a look if you have time. Better bug to start with than  issue 735269 
Cc: -lalitm@google.com
Owner: lalitm@chromium.org
Status: Started (was: Untriaged)
Thanks for the tip; working on this now!
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 21 2017

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

commit fdc0a261697b6e767f45acb3d61fb5dc005d205a
Author: Lalit Maganti <lalitm@chromium.org>
Date: Thu Sep 21 12:30:10 2017

memory-infra: Enable LevelDB global dumps in background mode

The existing LocalStorageContextMojo creates a leveldb dump that
does not have size. The size is added by LevelDBDatabaseImpl on the
service side using a global dump. Since we need this size to be
attributed to local storage, the background mode should support
global dumps edges.

This CL enable support for global dumps in background mode by removing
if checks and updating the whitelist to explicitly allow global dump
keys.

Bug:  762639 
Change-Id: I37ee050636529988acf56b220b3a892dbe45630b
Reviewed-on: https://chromium-review.googlesource.com/657383
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503422}
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/base/trace_event/process_memory_dump_unittest.cc
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/third_party/leveldatabase/env_chromium.cc
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/third_party/leveldatabase/env_chromium.h
[modify] https://crrev.com/fdc0a261697b6e767f45acb3d61fb5dc005d205a/third_party/leveldatabase/env_chromium_unittest.cc

Comment 4 by lalitm@chromium.org, Sep 25 2017

Status: Fixed (was: Started)
This should now be fixed!

Sign in to add a comment