674869 - Delay corrupt cache deletion to avoid startup I/O contention

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Jason Duell]

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.

Comment 1

[Jason Duell]

Attachment: Delays Cache.trash delete for 1 minute at startup

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.

Comment 2

[Michal Novotny [:michal]]

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?

Comment 3

[Jason Duell]

Attachment: Fixed from Michal's comments

> 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.

Comment 4

[Jason Duell]

http://hg.mozilla.org/integration/mozilla-inbound/rev/feda6619295c

Comment 5

[Marco Bonardo [:mak]]

has been pushed with the wrong bug number, luckily bugzilla has fulltext search...
http://hg.mozilla.org/mozilla-central/rev/feda6619295c