Flush localStore.rdf off the main thread

RESOLVED WONTFIX

Status

()

Core
RDF
RESOLVED WONTFIX
5 years ago
21 days ago

People

(Reporter: vladan, Assigned: wchen)

Tracking

(Blocks: 2 bugs, {main-thread-io})

Trunk
main-thread-io
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Snappy:P2])

(Reporter)

Description

5 years ago
We should look into flushing the localStore.rdf file off the main thread on shutdown:

http://people.mozilla.com/~bgirard/cleopatra/#report=1ecd7a421948995171a4bb483b7bcc8e1868cc57

Comment 1

5 years ago
I'm new to the topic, what would that mean? code wise, and in terms of "written file to disk before shutdown" etc.

If I read the profile right, we're actually spending time in ::Flush, i.e., not in RDF itself, but only in the actual finish of the disk write?

Is that true for various sizes of localstore.rdf?
@Axel: I believe that this is a general issue with flushing. Its duration is unbounded, regardless of the size of the file.

Updated

5 years ago
No longer blocks: 810156

Updated

5 years ago
Blocks: 819063
(Reporter)

Updated

5 years ago
Whiteboard: [Snappy:P2]
Keywords: main-thread-io
Blocks: 572459
Blocks: 987728
William's going to work on this.
Assignee: nobody → wchen
(Assignee)

Comment 4

23 days ago
I took a look into this and I don't think it's worthwhile to move Flush() off the main thread.

RDF serialization, where the Flush() spends most of its time, uses a lot of main-thread only objects (RDFService, RDFXMLDataSource, ContainerEnumerator, nsStandardURL and likely many more that I haven't discovered). For some of these objects, we can use runnables to do what we need on the main thread while other objects will probably have to be converted to be fully thread-safe. We don't have good test coverage of this code so such a major change is risky.

Also, this is legacy code and we don't use localStore.rdf for Gecko anymore. It looks like only extensions use this (and the problem only exhibits itself when a lot of data gets stored into localStore.rdf, causing subsequent flushes to be slow). This problem should also go away by itself in version 57 since extensions will no longer be able to write into the file.

For these reasons I'm marking this bug as WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 23 days ago
Resolution: --- → WONTFIX
Also see bug 987728 comment 11.  It turns out that the recent spike in signatures involving code around this in our BHR data was due to the TabMixPlus add-on, not anything caused by our code.
You need to log in before you can comment on or make changes to this bug.