686706 - Don't fsync when initializing nsDownloadManager

Status: RESOLVED WONTFIX

(Toolkit :: Downloads API, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0.2) Gecko/20100101 Firefox/6.0.2
Build ID: 20110902133214



Actual results:

At startup, nsDownloadManager uses sqlite in synchronous mode, which causes useless fsync when creating an empty database.


Expected results:

Remove the fsync.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Proposed patch

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Attachment: Same patch, reformated as per glandium's suggestion

Comment 3

[Shawn Wilsher :sdwilsh]

What happens if you crash then?

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Good point. I am just starting my review on startup I/O but currently, I have the impression that all sorts of things can go wrong if we crash at startup and that transacting nsDownloadManager init will not help.

Comment 5

[Shawn Wilsher :sdwilsh]

The right fix here is to make all the download manager APIs async.  That's a lot of work, though.

If that is undertaken, it may be worthwhile to look into implementing the download manager on top of Places (since almost all the data we care about is already stored there) instead of having its own database.

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Taking a look at sqlite async I/O (http://www.sqlite.org/asyncvfs.html).

At first glance, it seems to take the simple way towards async I/O: put all the I/O in a distinct thread. We could certainly use this, or, better, have a startup thread for all such startup I/O. I'll try working on the second option.

Comment 7

[Shawn Wilsher :sdwilsh]

No, we don't want to use SQLite's async I/O.  It throws away durability in ACID.  We have written our own async I/O for SQLite (see `executeAsync` on `mozIStorageStatement` and `mozIStorageConnection`).

Comment 8

[David Teller [:Yoric] - still alive but not very active]

First (well, second) draft of a mechanism that could possibly help with async initialization: https://wiki.mozilla.org/User:Yoric/RunLevelQueue .

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Attachment: Same patch, thanks Vlad for noticing the typo

Comment 10

[Marco Castelluccio [:marco]]

Does the patch need a review or it's still a work in progress?

Comment 11

[David Teller [:Yoric] - still alive but not very active]

It is a work in progress.

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Paolo, will bug 825588 et al. help here?

Comment 13

[:Paolo Amadini]

(In reply to David Rajchenbach Teller [:Yoric] from comment #12)
> Paolo, will bug 825588 et al. help here?

Definitely, that's more or less the "lot of work" mentioned in comment 5 :-)