Service worker database stores unnecessary data |
||||||||
Issue descriptionI found my service worker database was 2MB and printed the contents of the database. It seems to have the following keys: 8060 URES:__ 8060 PRES:__ 4020 INITDATA_NEXT_RESOURCE_ID 4020 REG:__ 25 RES:__ 8 INITDATA_NEXT_VERSION_ID 9 INITDATA_UNIQUE_ORIGIN 10 REGID_TO_ORIGIN:__ It feels like some website had spammed by registering so many service workers? Can we have a limit on this registration? Also storing INITDATA_NEXT_RESOURCE_ID looks like a bug?
,
Feb 8 2018
I'm not sure I understand the bug. Is it that 2 MB is unexpectedly high? Can you look in chrome://serviceworker-internals/ to see if you have unexpectedly many registrations? We must store INITDATA_NEXT_RESOURCE_ID, it's a counter that tells us the next available "resource ID". Resources (i.e., the service worker scripts themselves) are stored in a separate storage system (SimpleCache). Also, why is it Perf>Memory? Is this impacting memory?
,
Feb 8 2018
2MB is a high memory usage for Android devices. It also loads 2MB of database into memory at startup and lives in memory forever. that is why I added Perf-memory label. Reducing that could really help.
chrome://serviceworker-internals shows 6 entries. But the database holds 4000 or 8000 entries. There are 4k entries with "localhost". Can a single origin store 4000 service workers with the same script url?
Why does it store 2 entries of "URES" and "PRES" for each resource ("RES") entry?
Why do we need to store 4000 entries of "INITDATA_NEXT_RESOURCE_ID" instead of just the last one?
Note: I was trying out a sample service worker page on localhost and ended up with this state. But, a badly designed website could also cause this for users I guess.
I can paste a dump of my service worker database items if you like.
,
Feb 8 2018
+storage for LevelDB expertise Understood. I didn't understand the numbers in c#0, is that the number of entries? I don't expect us to be storing more than one entry of INITDATA_NEXT_RESOURCE_ID. I think what might have happened is LevelDB stores changes in a log and periodically prunes them?
,
Feb 8 2018
The database schema is here: https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_database.cc URES is for "uncommitted resources". We've stored the resource but the service worker has not yet installed. It gets purged later. PRES is for "purgeable resources". The service worker using the resource has been deleted or failed to install. It gets removed later.
,
Feb 8 2018
I don't understand how the numbers in c#0 were calculated either. It's leveldb, there is no way to have duplicate keys, at least not at the API level (of course until compaction happens at the disk level there might be duplicate keys, but that shouldn't be visible in any normal way).
,
Feb 8 2018
,
Feb 8 2018
"printed the contents of the database" - how? Did you perhaps run "strings" over the most recent *.log file? If I do that I get similar numbers (2012 INITDATA_NEXT_RESOURCE_ID, 4028 URES, 4262 PRES, etc). This is leveldb WAI - it appends log entries and then compacts when various thresholds are hit, as notes in comments 4 and 6. Options include not using leveldb, or not keeping the database resident in memory on low memory devices (at the cost of startup time).
,
Feb 8 2018
Thanks for explanation. You are right. I was just printing all the items in level db database in memory in #c0. This included the duplicate keys. It is not visible from api level. But the database still uses memory for these log entries. I called CompactRange() after loading database and the memory went down from 2MB to 70KB. Looking at the code for triggering compaction, and it has heuristics based on number of files? But my device didn't seem to have run compact in the past 2 weeks. Do you think it might be a good idea to compact manually say every time the NEXT_ID increased by 1000? The actual data stored in the database is not too much, so keeping in-memory should be ok. Is there a scope for optimizing the compaction threshold logic for low memory devices?
,
Feb 8 2018
,
Feb 8 2018
I think compaction (of new writes) happens when the write buffer is full. So with the default write_buffer_size of 4MB that ServiceWorkerDatabase is using that would mean it won't flush that until 4MB of data has been written to the log.
,
Feb 8 2018
In that case is it ok to lower the write buffer size for the service worker database? Since we are not expecting to store a lot of entries the the database, it should be ok for performance?
,
Feb 8 2018
Seems reasonable to me, yes.
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c73a3297df94f3ebbb3b3e322259fd261e347e55 commit c73a3297df94f3ebbb3b3e322259fd261e347e55 Author: Siddhartha <ssid@chromium.org> Date: Thu Feb 08 23:23:06 2018 Reduce the write buffer size of ServiceWorkerDatabase The users have 50 service worker registrations at 99.5th percentile. The expected memory required in the database is around 300KB for 50 registrations. Having a large write buffer only causes the database to store large log entries since the entries are overwritten frequently. Reduce the write buffer to trigger compaction more often. BUG= 810237 Change-Id: I334fb1ab8e21fbd30c5a2f46e647f3f56988e7dc Reviewed-on: https://chromium-review.googlesource.com/909856 Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Siddhartha S <ssid@chromium.org> Cr-Commit-Position: refs/heads/master@{#535570} [modify] https://crrev.com/c73a3297df94f3ebbb3b3e322259fd261e347e55/content/browser/service_worker/service_worker_database.cc
,
Feb 9 2018
(Service worker bug triaging) Do you think this is good to mark as fixed?
,
Feb 13 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by ssid@chromium.org
, Feb 8 2018Owner: falken@chromium.org