Closed
Bug 674869
Opened 13 years ago
Closed 13 years ago
Delay corrupt cache deletion to avoid startup I/O contention
Categories
(Core :: Networking: Cache, defect)
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)
References
(Blocks 1 open bug)
Details
(Whiteboard: [inbound])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
michal
:
review+
|
Details | Diff | Splinter Review |
Followup to bug 670911. Now that we've avoided UI hang, we should also be polite and avoid doing the (very significant for large HTTP caches) I/O to delete the cache, so that it doesn't block other IO done during startup.
I'm currently thinking about 2 minutes is a reasonable time.
I'm also leaning towards just deleting the cache in one go at that time. It will be noticable (see bug 670911 comment 20: may be 20 seconds or more of IO), but amortizing it slowly enough that it's not noticeable has other bad effects (re-spinning up notebook hard drives), and it's not clear to me that, say, 16 noticeable bursts of drive activity is going to be less irritating than one big one.
The correct long-term fix, of course, is to not have so many small files in the tree, and/or reuse existing files like the cache map files.
Assignee | ||
Comment 1•13 years ago
|
||
This patch also does some extra effort to make sure that users who get stuck in a cycle of crashes (with a stack of bad Cache dirs) can still make progress deleting the trash dirs if/when their browser stops the early crashing pattern.
Assignee: nobody → jduell.mcbugs
Status: NEW → ASSIGNED
Attachment #549273 -
Flags: review?(michal.novotny)
Comment 2•13 years ago
|
||
Comment on attachment 549273 [details] [diff] [review]
Delays Cache.trash delete for 1 minute at startup
> +static void CreateDeleterThread(nsITimer *aTimer, void *arg)
> +{
> + if (aTimer)
> + aTimer->Release();
Could you please explain the logic? IMO if you don't call timer.forget() in DeleteDir(), you won't need to call Release() here.
> +nsresult DeleteDir(nsIFile *dirIn, PRBool moveToTrash, PRBool sync,
> + unsigned long delay)
Why not PRUint32?
Attachment #549273 -
Flags: review?(michal.novotny)
Assignee | ||
Comment 3•13 years ago
|
||
> IMO if you don't call timer.forget() in DeleteDir(), you won't need to call Release() here.
You're right--I didn't realize the timer thread takes care of ensuring the timer is alive until it finishes firing.
> Why not PRUint32?
I spaced out and used the IDL type. Fixed.
thanks.
Attachment #549273 -
Attachment is obsolete: true
Attachment #549962 -
Flags: review?(michal.novotny)
Updated•13 years ago
|
Attachment #549962 -
Flags: review?(michal.novotny) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Comment 5•13 years ago
|
||
has been pushed with the wrong bug number, luckily bugzilla has fulltext search...
http://hg.mozilla.org/mozilla-central/rev/feda6619295c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•