Open Bug 1446012 Opened 3 years ago Updated 1 year ago
The first startup after an update does main thread I/O to update
.status several times
First there's a .exists() call at https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/toolkit/mozapps/update/nsUpdateServiceStub.js#30 Then https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/toolkit/mozapps/update/nsUpdateService.js#1610 reads the same file just for the sake of setting a pref. And then, the same code falls through to https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/toolkit/mozapps/update/nsUpdateService.js#1635 with a _postUpdateProcessing call, and that reads the file again at https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/toolkit/mozapps/update/nsUpdateService.js#1700 There are several more calls to readStatusFile that would be nice to audit, I wouldn't be surprised if a significant portion of them could be converted to async I/O.
(In reply to Florian Quèze [:florian] from comment #0) > Then > https://searchfox.org/mozilla-central/rev/ > 8976abf9cab8eb4661665cc86bd355cd08238011/toolkit/mozapps/update/ > nsUpdateService.js#1610 reads the same file just for the sake of setting a > pref. At least initially, it seems the problem is that quite a few things hang off of that pref. pdfjs initialization uses it, as well as our startup decision to load an update page. Especially that last thing happens very early on (in handling of default commandline arguments). It's not clear to me off-hand how we could fix that without forgoing the IO. Of course, we could consolidate the IO into 1 call that reads the file, but I'm skeptical that would make much of a performance difference because of disk caching, and if we would need to refactor to eliminate the sync IO completely then that refactor would likely eliminate the use of the optimization. Doug, certainly within the perf team I suspect you're the most familiar with our updater code... do you have any ideas for what scope we have to fix this by somehow communicating the update status and/or override page in a way that doesn't require early-startup sync IO?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dothayer)
Well, the file is always pretty small. It seems like we could pass it as an argument from the updater back to firefox.exe no? Other than that - do we have an intuition for how expensive this is, given that the updater just wrote to the file? I would expect it to be not so bad since it should be in the system file cache? : https://searchfox.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2080
Here is another more recent startup profile where the updater code does main thread I/O: https://perfht.ml/2LS415v The stack is: FileUtils_closeSafeFileOutputStream writeStringToFile writeStatusFile Downloader_downloadUpdate AUS_downloadUpdate AUS__postUpdateProcessing AUS_observe
Priority: -- → P2
Whiteboard: [fxperf:p2] → [fxperf:p2][fix suggestion in comment 8]
You need to log in before you can comment on or make changes to this bug.