712009 - Move localStorage writes out of mainthread

Status: RESOLVED DUPLICATE of bug 600307

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

Description

[Marco Bonardo [:mak]]

this can be done with a separate connection, I have an experimental patch, some edge still needs to be perfected.

Comment 1

[Marco Bonardo [:mak]]

Attachment: wip

this is largely incomplete, but contains some interesting changes related to partitioning that may help related bugs.

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 585560 [details] [diff] [review]
wip

ehr, wrong patch

Comment 3

[Marco Bonardo [:mak]]

Attachment: wip

Comment 4

[Marco Bonardo [:mak]]

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.

Comment 5

[Marco Bonardo [:mak]]

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?

Comment 6

[Marco Bonardo [:mak]]

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.

Comment 7

[Marco Bonardo [:mak]]

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.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 9

[Marco Bonardo [:mak]]

yes, is what this bug is about.

Comment 10

[Marco Bonardo [:mak]]

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.

Comment 11

[Honza Bambas (:mayhemer)]

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.

Comment 12

[Marco Bonardo [:mak]]

Thank you Honza.