686691 - PrefixSet loading should use readahead

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

The loading of the PrefixSet datastructure in the url-classifier is a linear +-1.2M read from disk. From Telemetry we see that nevertheless this is taking >1s for some users. Readahead could help here.

Comment 1

[(dormant account)]

Attachment: plumbing to expose os-level readhead

use something like 
rv = file->OpenNSPRFileDesc(PR_RDONLY | nsILocalFile::OS_READAHEAD, 0, &fileFd); 
This will cause os-level readahead to kickin, usually this means windows/linux will read the file in 2mb increments instead of <=128K.

Comment 2

[(dormant account)]

It might also help to use mozilla::fallocate when you write out the file(if you can predict the file size). If you can't predict an exact size you can allocate a bigger file and then ftruncate. This avoids fragmentation.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Expose OS-level readahead

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 2. Optimize for serial load/write

Comment 5

[Gian-Carlo Pascutto [:gcp]]

I fixed 2 things in your code:

- posix_fadvise isn't in Android's libc. I added checks for the relevant levels of POSIX conformance.
- #include "private/pprio.h" is required for PR_FileDesc2NativeHandle.

Comment 6

[(dormant account)]

Comment on attachment 561703 [details] [diff] [review]
Patch 2. Optimize for serial load/write

> 
> nsresult
> nsUrlClassifierPrefixSet::StoreToFd(AutoFDClose & fileFd)
> {
>+  PRInt64 size = 4 * sizeof(PRUint32);
>+  size += 2 * mIndexStarts.Length() * sizeof(PRUint32);
>+  size +=     mDeltas.Length() * sizeof(PRUint16);
>+
>+  mozilla::fallocate(fileFd, size);

One thing I forgot to mention. On crappy filesystems fallocate can be an expensive call. This needs to get a telemetry probe to see if this is expensive, then we can decide to turn this off on RFS/VFAT/etc.

Comment 7

[(dormant account)]

Attachment: Patch 1. Expose OS-level readahead

Comment 8

[(dormant account)]

Attachment: Patch 1. Expose OS-level readahead

Do not do readhead() on android. I will address fallocate & readahead for android in a followup.

Comment 9

[(dormant account)]

Attachment: Patch 1. Expose OS-level readahead

Sorry, didn't qref above.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 2. Optimize for serial load/write

Added Telemetry probe.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d7569eaa6ced

Comment 12

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/fd5e3cb57245
http://hg.mozilla.org/integration/mozilla-inbound/rev/eeeb4aa609f6

Comment 13

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/fd5e3cb57245
https://hg.mozilla.org/mozilla-central/rev/eeeb4aa609f6