Set DOM Storage buffer size so that not everything ends up in RAM

Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
Committed: https://crrev.com/e4286a6f1d07ede602eebbbe0e604306c1713534
Cr-Commit-Position: refs/heads/master@{#373780}

[Daniel Bratell, 2016-02-03 12:32:17]

Description was changed from

==========
Set DOM Storage write buffer so that not everything end up in RAM

BUG=583629
==========

to

==========
Set DOM Storage write buffer so that not everything end up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

[Daniel Bratell, 2016-02-03 12:32:17]

bratell@opera.com changed reviewers:
+ michaeln@chromium.org

[Daniel Bratell, 2016-02-03 12:32:49]

Please take a look. I absolutely do not know if this is the right way to handle
this but it worked(TM).

[Daniel Bratell, 2016-02-03 12:33:31]

Description was changed from

==========
Set DOM Storage write buffer so that not everything end up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

to

==========
Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

[michaeln, 2016-02-03 20:59:14]

michaeln@chromium.org changed reviewers:
+ cmumford@chromium.org

[michaeln, 2016-02-03 21:08:35]

Chris, can you take a look at this one? I'm not sure of all of the implications
of changing that buffer size. Does this value affect the max amount of data that
can be accrued in a write batch for example?

fyi: There is one SessionStorageDB per storagepartition so the number of
isntances of this class per running browser should be in the single digits.

[cmumford, 2016-02-03 21:34:51]

https://codereview.chromium.org/1668463003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/session_storage_database.cc (right):

https://codereview.chromium.org/1668463003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/session_storage_database.cc:388:
options.write_buffer_size = 64 * 1024;
64K is the minimum, so this will work, but it will slow down db writes.
michaeln@ what impact on performance do you think this CL will have?

[michaeln, 2016-02-03 22:00:40]

> content/browser/dom_storage/session_storage_database.cc:388:
> options.write_buffer_size = 64 * 1024;
> 64K is the minimum, so this will work, but it will slow down db writes.
> michaeln@ what impact on performance do you think this CL will have?

Idk, how much slower?

The doc comments say...

  // Amount of data to build up in memory (backed by an unsorted log
  // on disk) before converting to a sorted on-disk file.
  //
  // Larger values increase performance, especially during bulk loads.
  // Up to two write buffers may be held in memory at the same time,
  // so you may wish to adjust this parameter to control memory usage.
  // Also, a larger write buffer will result in a longer recovery time
  // the next time the database is opened.

Restoring a browsing session could be a little slower, how much, i couldn't
really say. Initial accesses to session storage from within pages could be
affected by slower bulk loads.

Flushing the db when the browser quits could take a little longer depending on
how much was pending. We get cutoff in some case, in more of them we'd could
lose some data during shutdown.

I don't really know large the impact would be in practice, but i think we may
have uma metrics that could tell us. It might be hard to tease them apart from
localstorage numbers?

[michaeln, 2016-02-03 22:03:03]

land it and watch the uma numbers?

[michaeln, 2016-02-03 22:14:07]

how about a compare the time it takes to run the SessionStorageDatabaseTest
unittests with and without the change to see if there's a difference large
enough to look into further?

[michaeln, 2016-02-03 22:17:41]

On 2016/02/03 22:14:07, michaeln wrote:
> how about a compare the time it takes to run the SessionStorageDatabaseTest
> unittests with and without the change to see if there's a difference large
> enough to look into further?

none of those tests populate an origin's area with more than a few values, maybe
add at least one test that deals with a larger amount of data (5m'ish)

[michaeln, 2016-02-04 00:31:37]

I ran some tests and timed them, release built SessionStorageDatabase*
unittests. It is definitely slower when working with large amounts of data,
about 2x slower. I added a test that works with 10M of data almost all of the
timing difference between the before and after runs are attributable to that one
test taking 2x as long.


I think this is ok, but i want to confirm that small operations on a large db
aren't impacted by a 2x factor. My tests doesn't tell me that yet.


TEST_F(SessionStorageDatabaseTest, MegaBytesOfData) {
  DOMStorageValuesMap data1;
  PopulateValueMap(&data1, 5, 1024 * 1024);  // 5 x 1 megabyte values
  ASSERT_TRUE(db_->CommitAreaChanges(kNamespace1, kOrigin1, false, data1));
  DOMStorageValuesMap data2;
  PopulateValueMap(&data2, 5000, 1024 * 1024);  // 5000 x 1 kilobyte values
  ASSERT_TRUE(db_->CommitAreaChanges(kNamespace2, kOrigin2, false, data2));

  // Close and reopen.
  ResetDatabase();

  CheckAreaData(kNamespace1, kOrigin1, data1);
  CheckAreaData(kNamespace2, kOrigin2, data2);

  EXPECT_TRUE(db_->DeleteArea(kNamespace1, kOrigin1));
  EXPECT_TRUE(db_->DeleteArea(kNamespace2, kOrigin2));

  // Check that also the namespace start key was deleted.
  CheckDatabaseConsistency();
}

[Daniel Bratell, 2016-02-04 09:34:31]

Let me see if I understand.

At some time the cost to move the data from the unsorted log file to the
database must be payed.

Having all the data unsorted all the time will just make startup time slow so
we'll pay the sorting cost every startup, but operations will be fast (working
on an in-memory database and a raw append-to-file).

Having a 64 KB write buffer (1.5% of the default 4 MB buffer) makes shuffling
large amounts of data twice as slow because we'll flush sorted data to disk very
often.

----

I don't know what the API allows but it seems the best would be to allow the
in-memory database to grow to a limit (maybe not 4MB but more than 64KB) during
intense work and then flush it when idle. That would keep startup time down and
RAM usage down while not affecting performance of db intense scripts.

Alternatively, can the startup buffer size be small (making the to-disk sorting
a one-time cost) and then increased while running? Depending on how the browser
is used, this might be a startup performance hit though since every startup will
pay the cost of not having stored the data sorted on disk the previous session.

As a third alternative, maybe 64 KB is just too aggressively. 128 KB, being
double the size, might be enough to almost double the speed (halve the number of
flushes) while still fixing a large part of the problem.

[michaeln, 2016-02-04 21:00:27]

Small ops on big databases aren't bothered by this change.

Did some more timing tests but with the extra expense of performing small
operations (1x 256 byte value) on a large (5M) database:
opendb/writeval/closedb/opendb/readval/deleteval/closedb.
Ran the test with default 4M write buffer and 64kb buffer, 38541ms 39647ms. The
delta is the same as the delta without the extra expenses added in.

The writebuffer size should only impact writes which won't be happening during
startup.

I think this is safe enough to change if we think the mem savings are worth it.

> I don't know what the API allows but it seems the best would be to allow the
> in-memory database to grow to a limit (maybe not 4MB but more than 64KB)
during
> intense work and then flush it when idle. That would keep startup time down
and
> RAM usage down while not affecting performance of db intense scripts.

I don't see an api for 'reset write buffer'.

[cmumford, 2016-02-04 22:55:26]

lgtm

[michaeln, 2016-02-04 22:56:23]

lgtm 2

[Daniel Bratell, 2016-02-05 08:59:01]

On 2016/02/04 22:56:23, michaeln wrote:
> lgtm 2

Thanks! If it turns out to cause any problem, I would just try doubling the
constant.

[Daniel Bratell, 2016-02-05 08:59:06]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2016-02-05 08:59:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1668463003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1668463003/1

[commit-bot: I haz the power, 2016-02-05 11:05:02]

Description was changed from

==========
Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

to

==========
Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

[commit-bot: I haz the power, 2016-02-05 11:05:02]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-05 11:05:57]

Description was changed from

==========
Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629
==========

to

==========
Set DOM Storage buffer size so that not everything ends up in RAM

When a database is opened the log file is read and if the write
buffer is large then it stays in memory, and this happens
every time the browser starts. By reducing the write buffer
the log file will be written into the database the first time
and then very little RAM will be used for that data.

BUG=583629

Committed: https://crrev.com/e4286a6f1d07ede602eebbbe0e604306c1713534
Cr-Commit-Position: refs/heads/master@{#373780}
==========

[commit-bot: I haz the power, 2016-02-05 11:05:57]

Patchset 1 (id:??) landed as
https://crrev.com/e4286a6f1d07ede602eebbbe0e604306c1713534
Cr-Commit-Position: refs/heads/master@{#373780}