896885 - Enable localStorage to do IO in child process

Status: RESOLVED INVALID

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

Description

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

Right now localStorage always proxies calls to the parent process when reads/write to/from the filesystem is happening. This has a couple of problems.

First off it means that we're putting more load on the parent process as far as CPU usage goes.

Second, it means that when we do the IO, we end up with multiple copies of the data in memory at the same time. At least two of which are in the parent process. First the data in sqlite. Then the copy in ipdl as we send the data to the child process. Then either the parent or the child (or both?) caches for future reads.

We should enable the child process to instead do the IO directly. For now that means that the parent process needs to open the file handle, and then clone it to the child process. This since child processes doesn't have access to open files in B2G.

In order to do this it means that we have to split localStorage so that separate apps get separate files. Otherwise child processes can access the localStorage data of other apps. This is something we need to do anyway in order to integrate localStorage with the new quota manager though. Probably that should be filed as a separate bug blocking this.

Comment 1

[Honza Bambas (:mayhemer)]

For what version of the DOM storage code is this intended?  For the new or for the old?

In the new code, it should be fairly simple just by changing a condition at [1] to always instantiate DOMStorageDBThread and provide the profile dir also on the child process (NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR)).

In the old code, it can be done in a similar fashion at [2], but I don't know what all other should change to make this really work.


[1] http://hg.mozilla.org/mozilla-central/annotate/b717a7945dfb/dom/src/storage/DOMStorageCache.cpp#l746
[2] https://hg.mozilla.org/releases/mozilla-b2g18/annotate/68fb0a2e0114/dom/src/storage/nsDOMStorage.cpp#l1102

Comment 2

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

This is only intended for the new code.

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Jonas Sicking (:sicking) from comment #2)
> This is only intended for the new code.

Good.

I've realized one more thing.  We need the IPC (that the child side implementation of the database interface btw provides) to get clear notification from the chrome process like "clear cookies", "clear app data", etc.  So we will need to instantiate both the "child DB" to have the bridge to the chrome process AND the full database to access sqlite directly.

Comment 4

[Andrew Overholt [:overholt]]

Jan, is this contrary to your new design?

Comment 5

[Jan Varga [:janv]]

Well, my new design does I/O only in the parent. We do the same for IndexedDB. Quota manager is also currently limited to do I/O in the parent process. If there's evidence that this is a performance problem, we should start with QM, IDB, etc.

Comment 6

[Marion Daly [:mdaly]]

@jan Think we should close this bug>?

Comment 7

[Jan Varga [:janv]]

Yeah, we will have separate database files after new LS lands, so the proposal in comment 0 would be possible easier to do, but with centralized database and cache management in the chrome process, it's possible to do various other optimizations.
Especially, we don't have to synchronize cached data across separate processes (for the same origin).
Just for reference, upsides and downsides of the new architecture are described in bug 1286798 comment 8.

I think this bug was filed when B2G was still a thing and we had processes per app, and the hardware had limited RAM.

So yes, I think we can close this bug.

Andrew, do you have any additional comments for this ?

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

I agree this was primarily a B2G-era consideration.  We would need the invariant that there would only be 1 process per origin for this to simplify things, and that's not currently an invariant the fission project is guaranteeing per in-person discussion with :nika in Toronto last week.

Resolving invalid because this is now moot.