Closed
Bug 712009
Opened 13 years ago
Closed 12 years ago
Move localStorage writes out of mainthread
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
DUPLICATE
of bug 600307
People
(Reporter: mak, Unassigned)
References
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
this can be done with a separate connection, I have an experimental patch, some edge still needs to be perfected.
Reporter | ||
Comment 1•13 years ago
|
||
this is largely incomplete, but contains some interesting changes related to partitioning that may help related bugs.
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 585560 [details] [diff] [review]
wip
ehr, wrong patch
Attachment #585560 -
Attachment is obsolete: true
Reporter | ||
Comment 3•13 years ago
|
||
Reporter | ||
Comment 4•13 years ago
|
||
The idea is 2 connnection, one synchronous and read-only, one asynchronous.
The data for the accessed scope is copied into the read-only one, that works like a memory cache. Updates happen on both connections, those to disk are written asynchronously to disk, while others only persist in memory. The data in the memory cache should be removed after a certain amount of time it's unmodified, that ensures the disk write happened already (it would be possible to enforce a better check, but it's not worth it, imo).
A drawback is that on delete of all data, it should be copied completely to the memory cache with a "deleted" flag, though it may possible to avoid that having a deleted-scopes hash and when data is copied from disk to memory copy it with the deleted flag set.
Reporter | ||
Comment 5•13 years ago
|
||
Honza, Taras had an idea regarding this, we could use a ScriptBlocker to actually pause the script while we asynchronously copy the data to the memory cache.
This would LARGELY simplify my approach here, to a point of making it trivial.
Do you think we may do something like that? Would that be against the specs?
Reporter | ||
Comment 6•13 years ago
|
||
Honza, ping?
One issue related to this is that we also use DOMStorage for some chrome consumers, like about:home (maybe also about:newtab), though those may survive with it being asynchronous since setting happens at a different time.
Reporter | ||
Comment 7•13 years ago
|
||
I have moved the ScriptBlocker idea to bug 728197. There is some pitfall there, like the fact in the past DOMStorage perf were improved to avoid blocking the page, and now we would block the page on purpose. I'm not even sure if DOMStorage is used in some benchmark out there.
There is another possibility we didn't consider so far, that is to spin the events loop on cache population. So the first time a scope is accessed we asynchronously populate the cache, but spin the events loop to make it synchronous. Would solve most of the complication in this patch, though it's something we are prone to avoid.
I think I will finish this patch regardless, since it's still a valid approach with no downsides apart some code complexity.
I suspect spinning the event loop on either reads or writes would break JSs run-to-completion model too much. This would result in getting things like XHR events possibly firing any time a page touches localStorage.
Instead we should minimize the number of times that we need to do synchronous disk IO. We should do this by caching the localStorage on the first read, and make writes asynchronous, which if I understand things correctly, is what this bug is about.
Reporter | ||
Comment 9•13 years ago
|
||
yes, is what this bug is about.
Reporter | ||
Comment 10•12 years ago
|
||
not actively working on this, Vladan has a different approach, plus there's some expected cleanups on DOMStorage code from Honza.
My approach was good looking, but far complicated having to sync up across 2 connections, I'd vote for such an approach only if we'd decide to block content loading on first localStorage access, then a pure async storage approach would be much better.
Assignee: mak77 → nobody
Status: ASSIGNED → NEW
Comment 11•12 years ago
|
||
I think this can duplicate then to bug 600307. I already have most of the patch for it done and database is fully accessed on a new background thread and is fully asynchronous in both reads and writes.
Reporter | ||
Comment 12•12 years ago
|
||
Thank you Honza.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•