Closed Bug 917012 Opened 6 years ago Closed 6 years ago
too many mainthread stat() calls during downloads
(In reply to Marco Castelluccio [:marco] from comment #1) > Bug 827010? There is no reason to do any syscalls here. We should have file size in gecko already.
(In reply to Taras Glek (:taras) from comment #2) > (In reply to Marco Castelluccio [:marco] from comment #1) > > Bug 827010? > > There is no reason to do any syscalls here. We should have file size in > gecko already. err...we should know that the file exists by nature of it being downloaded :)
I should note that I had the downloads window open in background...but was interacting with main window and hitting these .exists janks
From an IRC discussion: <gavin> seems to be a dupe of bug https://bugzilla.mozilla.org/show_bug.cgi?id=827010 <firebot> Bug 827010 nor, --, ---, nobody, NEW, In download views, use async I/O for retrieving data about the target and part files of downloads <gavin> referenced in the code <taras> gavin: right, except for the fact that we shouldnt stat() things we know the size of <gavin> well it's not about knowing the size <gavin> it's about the existence on disk <gavin> we hide/disable the "Open file" menu option if you've since deleted the downloaded file, e.g. <taras> gavin: sorry, same thing :) <taras> we know a file exists if we are downloading it <taras> :) <gavin> this code shouldn't be called during a download <gavin> well, I guess it could, if that results in selection changes <taras> gavin: it's called periodically <taras> without any interactions with download stuff <gavin> that seems like the bug
This happens when the state of a download changes, if the Library window or the in-content Downloads view is open. Bug 827010 was filed to fix that, assuming the file exists right after completion, and using OS.File to check for its existence later.
It turned out that bug 827010 was exacerbated by the fact that we sent onStateChange notifications too often. This patch fixes the regression introduced by the new Downloads API, while it leaves the general stat() call issue to bug 827010. It might be that that bug will be addressed as part of the refactoring to use the Downloads API without DataItem wrappers.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #809874 - Flags: review?(enndeakin)
Summary: Pointless mainthread stat() calls during downloads → too many mainthread stat() calls during downloads
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 809874 [details] [diff] [review] The patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 847863 User impact if declined: Severe jank when Library window open during downloads Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): Tested manually String or IDL/UUID changes made by this patch: None
Attachment #809874 - Flags: approval-mozilla-aurora?
Attachment #809874 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.