IndexedDB: Limit the size of a leveldb write buffer to minimize disk space.

IndexedDB: Limit the size of a leveldb write buffer to minimize disk space.

IndexedDB uses leveldb's default write buffer size of 4MB. This is usually
sufficient, but for space constrained devices, like Chromecast, this can
limit the amount of usable space because compaction requires double the
table space plus the logfile. Limiting the write buffer size reduces the
logfile size.

BUG=637847
Committed: https://crrev.com/edb3be685d259c8a42dfaeb1b87f7a32707dc28a
Cr-Commit-Position: refs/heads/master@{#412671}

[cmumford, 2016-08-15 18:59:35]

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

[cmumford, 2016-08-15 20:28:28]

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

[cmumford, 2016-08-15 20:28:28]

Switching reviewers to michaeln.

[michaeln, 2016-08-16 00:28:06]

lgtm!

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
File third_party/leveldatabase/env_chromium_unittest.cc (right):

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
third_party/leveldatabase/env_chromium_unittest.cc:30: extern size_t
WriteBufferSize(int64_t disk_space);
Is including env_chromium.h not enough, the function is declared in there?

[cmumford, 2016-08-16 00:54:26]

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
File third_party/leveldatabase/env_chromium_unittest.cc (right):

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
third_party/leveldatabase/env_chromium_unittest.cc:30: extern size_t
WriteBufferSize(int64_t disk_space);
On 2016/08/16 00:28:05, michaeln wrote:
> Is including env_chromium.h not enough, the function is declared in there?

I overloaded WriteBuferSize and this is the version that takes the disk space
(not the file/dir path). I did that to keep the public API clean. Still OK with
this approach, or do you think making this a public function is better?

[michaeln, 2016-08-16 19:41:23]

oh, the hidden overload is sneaky

https://codereview.chromium.org/2243363002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/leveldb/leveldb_database.cc (right):

https://codereview.chromium.org/2243363002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/leveldb/leveldb_database.cc:129:
options.write_buffer_size = leveldb_env::WriteBufferSize(path);
or maybe only expose the xxxForDiskSize() function in env.h and call
base::SysInfo::AmountOfTotalDiskSpace(path) here at the callsite

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
File third_party/leveldatabase/env_chromium_unittest.cc (right):

https://codereview.chromium.org/2243363002/diff/1/third_party/leveldatabase/e...
third_party/leveldatabase/env_chromium_unittest.cc:30: extern size_t
WriteBufferSize(int64_t disk_space);
On 2016/08/16 00:54:26, cmumford wrote:
> On 2016/08/16 00:28:05, michaeln wrote:
> > Is including env_chromium.h not enough, the function is declared in there?
> 
> I overloaded WriteBuferSize and this is the version that takes the disk space
> (not the file/dir path). I did that to keep the public API clean. Still OK
with
> this approach, or do you think making this a public function is better?

or maybe expose both functions in the .h file instead and give them different
names? WriteBufferSizeForDiskSize and WriteBufferSizeForPath or something like
that?

[cmumford, 2016-08-17 21:54:20]

The CQ bit was checked by cmumford@chromium.org

[cmumford, 2016-08-17 21:54:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/2243363002/#ps20001
(title: "Eliminated overloaded WriteBufferSize.")

[commit-bot: I haz the power, 2016-08-17 21:54:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 22:34:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-17 22:38:01]

Description was changed from

==========
IndexedDB: Limit the size of a leveldb write buffer to minimize disk space.

IndexedDB uses leveldb's default write buffer size of 4MB. This is usually
sufficient, but for space constrained devices, like Chromecast, this can
limit the amount of usable space because compaction requires double the
table space plus the logfile. Limiting the write buffer size reduces the
logfile size.

BUG=637847
==========

to

==========
IndexedDB: Limit the size of a leveldb write buffer to minimize disk space.

IndexedDB uses leveldb's default write buffer size of 4MB. This is usually
sufficient, but for space constrained devices, like Chromecast, this can
limit the amount of usable space because compaction requires double the
table space plus the logfile. Limiting the write buffer size reduces the
logfile size.

BUG=637847

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

[commit-bot: I haz the power, 2016-08-17 22:38:02]

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