Closed Bug 712009 Opened 13 years ago Closed 12 years ago

Move localStorage writes out of mainthread

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 600307

People

(Reporter: mak, Unassigned)

References

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

this can be done with a separate connection, I have an experimental patch, some edge still needs to be perfected.
Depends on: 714964
Attached patch wip (obsolete) (deleted) — Splinter Review
this is largely incomplete, but contains some interesting changes related to partitioning that may help related bugs.
Comment on attachment 585560 [details] [diff] [review] wip ehr, wrong patch
Attachment #585560 - Attachment is obsolete: true
Attached patch wip (deleted) — Splinter Review
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.
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?
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.
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.
yes, is what this bug is about.
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
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.
Thank you Honza.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: