Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1044)

Issue 1668463003: Set DOM Storage buffer size so that not everything ends up in RAM (Closed)

Created:
4 years, 10 months ago by Daniel Bratell
Modified:
4 years, 10 months ago
Reviewers:
michaeln, cmumford
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -0 lines) Patch
M content/browser/dom_storage/session_storage_database.cc View 1 chunk +3 lines, -0 lines 1 comment Download

Messages

Total messages: 23 (7 generated)
Daniel Bratell
Please take a look. I absolutely do not know if this is the right way ...
4 years, 10 months ago (2016-02-03 12:32:49 UTC) #3
michaeln
Chris, can you take a look at this one? I'm not sure of all of ...
4 years, 10 months ago (2016-02-03 21:08:35 UTC) #6
cmumford
https://codereview.chromium.org/1668463003/diff/1/content/browser/dom_storage/session_storage_database.cc File content/browser/dom_storage/session_storage_database.cc (right): https://codereview.chromium.org/1668463003/diff/1/content/browser/dom_storage/session_storage_database.cc#newcode388 content/browser/dom_storage/session_storage_database.cc:388: options.write_buffer_size = 64 * 1024; 64K is the minimum, ...
4 years, 10 months ago (2016-02-03 21:34:51 UTC) #7
michaeln
> content/browser/dom_storage/session_storage_database.cc:388: > options.write_buffer_size = 64 * 1024; > 64K is the minimum, so this ...
4 years, 10 months ago (2016-02-03 22:00:40 UTC) #8
michaeln
land it and watch the uma numbers?
4 years, 10 months ago (2016-02-03 22:03:03 UTC) #9
michaeln
how about a compare the time it takes to run the SessionStorageDatabaseTest unittests with and ...
4 years, 10 months ago (2016-02-03 22:14:07 UTC) #10
michaeln
On 2016/02/03 22:14:07, michaeln wrote: > how about a compare the time it takes to ...
4 years, 10 months ago (2016-02-03 22:17:41 UTC) #11
michaeln
I ran some tests and timed them, release built SessionStorageDatabase* unittests. It is definitely slower ...
4 years, 10 months ago (2016-02-04 00:31:37 UTC) #12
Daniel Bratell
Let me see if I understand. At some time the cost to move the data ...
4 years, 10 months ago (2016-02-04 09:34:31 UTC) #13
michaeln
Small ops on big databases aren't bothered by this change. Did some more timing tests ...
4 years, 10 months ago (2016-02-04 21:00:27 UTC) #14
cmumford
lgtm
4 years, 10 months ago (2016-02-04 22:55:26 UTC) #15
michaeln
lgtm 2
4 years, 10 months ago (2016-02-04 22:56:23 UTC) #16
Daniel Bratell
On 2016/02/04 22:56:23, michaeln wrote: > lgtm 2 Thanks! If it turns out to cause ...
4 years, 10 months ago (2016-02-05 08:59:01 UTC) #17
commit-bot: I haz the power
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
4 years, 10 months ago (2016-02-05 08:59:33 UTC) #19
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-02-05 11:05:02 UTC) #21
commit-bot: I haz the power
4 years, 10 months ago (2016-02-05 11:05:57 UTC) #23
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/e4286a6f1d07ede602eebbbe0e604306c1713534
Cr-Commit-Position: refs/heads/master@{#373780}

Powered by Google App Engine
This is Rietveld 408576698