485230 - Add way to disable caching during development

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Jason Duell]

Attachment: Disable cache if NECKO_DEV_DISABLE_CACHE env var set

Right now caching is on by default during development builds (if debug is on).  This can actually get in the way of writing tests, as the network traffic doesn't actually happen, and changes to header files, etc., may not happen.  Confusing.

I've added a patch that turns off caching in a development tree if NECKO_DEV_DISABLE_CACHE is set in the environment.   This seems better than turning it off during development by default, as we've got some cache tests that will presumably not get exercised (?) if caching is off.

I've taken away the #if DEBUG wrapper, so one effect of this patch is that caching will not be on by default for non-debug development trees.  Is that OK?

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

We also have a memory cache. That means:
- The tests that exercise caching do work, they use the memory cache (with a few special exceptions that set a profile dir or something, I think). Tinderboxes doesn't run tests in Debug mode AFAIK, so the tests need to support that :)
- This env variable is not named ideally, as it only affects the disk cache
- I don't think we should enable the disk cache in this case in opt builds. It has the effect that everyone running an opt xpcshell build will get Cache directories scattered around.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

There's actually an even better reason not to enable this by default: Embeddors who don't set a profile should not get a disk cache directory forced upon them.

Comment 3

[Jason Duell]

It sounds like you're saying we should never turn on disk caching by default in this block of code (i.e. if we don't find the parent cache directory using "regular" means), under either debug or opt builds, as debug builds will test caching via the memory cache, and opt builds don't want the disk cache directories lying around.  So I would change this patch to only turn on caching if some env var was set.  Right?

Sounds good in theory, but I'm a bit confused by the memory cache.  I'm loading a small, simple page, with no cache policy headers (so it should be cached, and it is if disk caching is on).  When I turn off disk caching, I then always see network traffic.  If memory caching is on, shouldn't we hit the memory cache instead of going over the network?  This is the one and only page load I do, from a freshly re-started browser.

> I've taken away the #if DEBUG wrapper, so one effect of 
> this patch is that caching will not be on by default for 
> non-debug development trees.  Is that OK?

Typo:  I should have said that caching *would* be on for non-debug development trees.  But you understood that that's what the code was doing.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #3)
> It sounds like you're saying we should never turn on disk caching by default in
> this block of code (i.e. if we don't find the parent cache directory using
> "regular" means), under either debug or opt builds, as debug builds will test
> caching via the memory cache, and opt builds don't want the disk cache
> directories lying around.  So I would change this patch to only turn on caching
> if some env var was set.  Right?

That's exactly right.

> Sounds good in theory, but I'm a bit confused by the memory cache.  I'm loading
> a small, simple page, with no cache policy headers (so it should be cached, and
> it is if disk caching is on).  When I turn off disk caching, I then always see
> network traffic.  If memory caching is on, shouldn't we hit the memory cache
> instead of going over the network?  This is the one and only page load I do,
> from a freshly re-started browser.

Yes. I'm pretty sure that's how the code is supposed to work. Might be interesting to see an nsHttp:5 log of that.

Comment 5

[Jason Duell]

Attachment: New fix

OK, I've change the patch so that we now only turn on caching in the build tree if NECKO_DEV_ENABLE_DISK_CACHE is set in the environment.  (I also got rid of some compiler warnings by changing some index variable to unsigned ints).

I tested the memory cache, and it works as it should, at least when I remember to turn on caching in my about:config :)

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 371715 [details] [diff] [review]
New fix

+        // use file cache in build tree only if asked, to avoid cache dir litter

it's the current directory... doesn't have to be the build tree

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #5)
> I tested the memory cache, and it works as it should, at least when I remember
> to turn on caching in my about:config :)

That'd certainly explain caching problems :)

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/73e08f744e9a