leveldb configuration

[Dan Harrington]

storage-dev@,

I've been tracking a memory regression introduced by a feature of ours. The feature itself isn't too important here, what I found was that our feature increase the usage of certain leveldb databases, and it was those databases that increased the memory usage of Chrome. 

There's three databases impacted by our feature, two owned by ntp_snippets, and one owned by the download component. All three databases typically store relatively few items, say maybe a dozen. As items are added and deleted, it results in higher memory usage. Since we're targeting Android devices which have limited memory, this is important.

I think the leveldb options for these databases needs updated. I went ahead and created a simple program that writes some data to a leveldb and then deletes that data. I ran that program 1000 times, reopening the database each time and writing and deleting 20KB worth of data (so that the database actually has no live data after each run).

I varied some of the leveldb options to see how memory and CPU time were affected.

Options:

reuse_logs (defaults to true)

max_file_size (2MB default)

write_buffer_size (4MB default)

Data is here:

https://docs.google.com/spreadsheets/d/1C5DEkUwXbIDEvctjFmobHLKvKohaxB5nqSPAalzW1V8/edit?usp=sharing

Some observations:

reuse_logs=true costs more in memory and time for low usage databases.

since there's many leveldb instances in Chrome, the defaults of 4MB/reuse_logs add up significantly to memory usage over time, even for "small" databases.

So I think I should change these three specific DBs to use much smaller write buffers, and to use reuse_logs=false. Does that make sense to you? Are there any risks?

Should we do something to make this poor configuration less likely in the future?

Is this the right group to field these questions :-)

Thanks!

[dew...@google.com]

+dtrainor for downloads

[David Trainor]

+Tommy Nyquist who cowrote the leveldb_proto code and has had talks about this as well with the leveldb owners.  Tommy do you know if storage-dev@ is the right list/is there someone else to rope in?  Thanks!

From what I remember, Chrome having a bunch of smaller leveldbs isn't really expected.  IIUC most things that use leveldb have a namespace_<identifier> for the DB and a single DB for the whole app.  If we're using it differently we probably should tweak those constants (or find a different way to do this in Chrome).

IIUC trace event should track leveldb as well, although I don't know if it tracks those caches you're talking about.  We should add that to the memory tracing if it doesn't so we can catch this easier in the future as well.

+Xing Liu  for FYI.

[Justin DeWitt]

Just fyi for the @google addresses here, this email thread cc's public lists.

[mariak...@google.com]

+ cmumford, who's specifically looked at tweaking settings for level_db for memory performance

[Maria Khomenko]

[Chris Mumford]

Hi Dan:

I think that setting reuse_logs=false is the way to go here. However, be careful to not make the write buffer size too small. Whenever the log gets full then a compaction is triggered. The min size is 64 KiB.

[Dan Harrington]

Thanks. I'm sending a CL to change the databases I've looked at specifically (https://chromium-review.googlesource.com/c/chromium/src/+/1026164).

Is someone planning on auditing the other databases? This could be one reason why Chrome appears to use more memory for older installs (age 1, 2, 3, 4, 5)

--

Dan

[Dan Harrington]

For those interested, this change has rolled out to canary (starting at 68.0.3410.0). There is a noticeable reduction of leveldb memory usage: 788 KB at the median for all users (we expect this to be even greater for users of our feature).

I'd like to see if we can get this merged for M67, but I need someone to vouch for this being a safe change. Any volunteers? 

The commit is here: https://chromiumdash.appspot.com/commit/a6ed7cd500316596deeae48069fb800fe70f8c52

UMA dashboard comparison: This Saturday, Previous Saturday

--

Dan

[Maria Khomenko]

Wow, that's a nice result. I am happy to support merge to M-67 for this one.

[Siddhartha S]

I am surprised that the download suggestions database was a major contributor to the leveldb memory usage. If this is expected then can we record a separate UMA for the service? Memory.Experimental.Browser2.Small.DownloadService doesn't report everything I guess?

Also noticed that the leveldb usage went down after this change and went back up again in few days:

https://uma.googleplex.com/p/chrome/timeline_v2/?sid=1900b0bcee3a80b06d07df0435476807.

[Dan Harrington]

> I am surprised that the download suggestions database was a major contributor

I noticed that when reuse_logs is used (the default), the leveldb database gets bigger until it hits the limit. I could not verify that the download database was one of the sources of memory usage. I only chose to 'fix' databases configs for databases that our feature could impact -- downloads and NTP suggestions.

> Also noticed that the leveldb usage went down after this change and went back up again in few days:

This very well could be another database. Maybe this one, it looks new and uses the default options? https://cs.chromium.org/chromium/src/chrome/browser/resource_coordinator/leveldb_site_characteristics_database.cc?l=74

Seems like we should have UMA reporting database stats for each individual database (or maybe we have this and I don't know where it is?).

--

Dan

[David Trainor]

For some more information on download database expected size (not including magic config overhead!), we have some metrics here that list the number of entries stored in the database.  Most are in the complete state and we are waiting to remove them (probably happens right after this loading metric is taken and we detect the files have been moved out by the prefetch logic).