1471668 - stop hitting the OS/disk so many times for a single preferences file write

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Nathan Froyd [:froydnj]]

This example is motivated by preferences, but it's basically the fault of xpcom/io/ and netwerk/base/, so I'm sticking it in XPCOM.  There are a bunch of problems here, so maybe this should be a meta bug of some sort?

I have a Windows profile (WPR/WPA) that shows us making a single call to PreferencesWriter::Write:

https://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#2815

Just looking at "create"-type operations, we do *54* of them within this call, split 2:1 between NS_NewSafeLocalFileOutputStream and BufferedOutputStream::Finish.  "create"-type operations aren't all calls to CreateFile, necessarily; they also apparently include GetFileAttributes and similar.  All the operations account for roughly a third of the time we spend in PreferencesWriter::Write.

Let's start with NS_NewSafeLocalFileOutputStream.  We drill down through this to nsFileStreamBase::DoOpen -> nsLocalFile::Create, where we attempt to create each and every directory along the path to said file.  This file resides in my profile directory, so that's:

C:\Users\froydnj\AppData\Roaming\Mozilla\Firefox\Profiles\$blah.default

...seven calls, from:

https://searchfox.org/mozilla-central/source/netwerk/base/nsFileStreams.cpp#337-346

I (mostly) understand the rationale for doing this, but surely we can do better.

But wait, that was from nsFileStreamBase::DoOpen, called from nsAtomicFileOutputStream::DoOpen:

https://searchfox.org/mozilla-central/source/netwerk/base/nsFileStreams.cpp#851

Earlier in that same function, we called CreateUnique:

https://searchfox.org/mozilla-central/source/netwerk/base/nsFileStreams.cpp#841-843

which actually *repeats* all the lameness of ::Create (by virtue of calling ::Create()), above, except that it does it *twice*, presumably to figure out a suitable temporary filename.  To attempt to create each intermediate directory three times is just dumb, and we should find a way to make that better.

We also have some twiddling around with Exists/GetPermissions/IsWritable/IsExecutable, which contribute their own file system accesses:

https://searchfox.org/mozilla-central/source/netwerk/base/nsFileStreams.cpp#807-839

GetPermissions looks like it refreshes the stat cache, which IsWritable and IsExecutable completely ignore, which is problematic.

nsBufferedOutputStream::Finish is almost reasonable in contrast, since all the accesses it generates are done by Windows itself (MoveFileExW/GetDriveTypeW, the latter of which is called by nsLocalFileWin::CopySingleFile -> IsRemoteFilePath)...which is really unfortunate.  Maybe we could figure out how to make those better too.

I'm assuming similar, though not identical, lameness happens on Unix, too.

All of this is happening off-main-thread, but it's still consuming resources that we don't need to be consuming, and given the general lameness of our preferences writing (and anything else that goes through NS_SafeLocalFileOutputStream), we're wasting all of this time quite often.