gordon Assignee
	Description • 24 years ago

Before data can take advantage of the cache block files, we need to implement a 
stream wrapper class that will be able to determine where best to store the 
incoming stream.

	gordon Assignee
	Comment 1 • 24 years ago

Stream wrappers are also necessary to implement over-lapped i/o for the disk 
cache.

	Gagan
	Comment 2 • 23 years ago

can't make it to 0.9.2. pushing over...

	gordon Assignee
	Comment 3 • 23 years ago

I'm hoping to get time to finish this for 0.9.4.

	gordon Assignee
	Comment 4 • 23 years ago

Won't get done for 0.9.6. Necko is focusing on performance for 0.9.7.

	gordon Assignee
	Comment 5 • 23 years ago

Made progress, but it's unlikely this will make 0.9.9.  We can pull it back if
it gets done earlier.

	Jaime Rodriguez, Jr.
	Comment 6 • 23 years ago

What are the chances this will be ready by 04/01? Can we get an ETA on this one?

	scottputterman
	Comment 7 • 23 years ago

Is this still on track to land on Friday, 4/5, pending approval?

	Gagan
	Comment 8 • 23 years ago

This bug blocks 122597 which is adt2. Considering the impending cleanup I'm
moving this up to adt2. 

	gordon Assignee
	Comment 9 • 23 years ago

The patch still has two known minor bugs to fix, but it should be good enough
for performance testing against page-loader and iBench.

Makefile changes still need to be made to include nsDiskCacheStreams.cpp in
build.

	gordon Assignee
	Comment 11 • 23 years ago

This patch does NOT contain the reference counted lock class for the disk cache
and streamIOs, and it doesn't resolve the lock order issues to enable dooming
of partial cache entries resulting from errors.

	Jaime Rodriguez, Jr.
	Comment 12 • 23 years ago

what are the chances this is gonna make Mozilla 1.0.1?

	Darin Fisher
	Comment 13 • 23 years ago

jaimejr: i'm hoping to find some cycles to finish off this patch ASAP.  i'd
really like to see it land for RTM, but i'm not sure yet if it's going to be a
reality.

	Cathleen
	Comment 14 • 23 years ago

If we can get a finalized patch, I can help getting some performance #s on it.  

	Darin Fisher
	Comment 15 • 23 years ago

cathleen: the current patch is ready for performance testing... there are just a
couple known multithreaded race-conditions that need to be closed before this
will be ready for prime time :)  the final patch shouldn't differ much
performance-wise

	Jaime Rodriguez, Jr.
	Comment 16 • 23 years ago

spoke to darin, and he didn't think this would make 1.0.1, so nsbeta1-. plus,
Gordon's on sabatical.

should the milestone for this one be moved to mozilla1.1beta?

	Jaime Rodriguez, Jr.
	Comment 17 • 23 years ago

removing [adt2 rtm] [ETA ?], as well

	Cathleen
	Comment 18 • 23 years ago

patch crashes on windows... so it's not yet ready.

	André Dahlqvist
	Comment 19 • 22 years ago

Gordon, do you have an updated patch for us to try?

	gordon Assignee
	Comment 20 • 22 years ago

I'm back from sabbatical, and I'll probably post an updated patch in the next
day or so.

	gordon Assignee
	Comment 21 • 22 years ago

I ran the page-load test and the minibench test with this patch, and I was able
to  shutdown without crashing.	I've tested on Mac, but we need to build and
test on the other platforms now.  We also need timing tests for a variety of
platforms.  I don't expect this will make much difference on Linux (other than
possibly startup time after a crash), but previous tests showed the a 500 MHz
Mac improving about 4%-7%.  It will be interesting to see what we get on
various Windows machines.

This patch increments the cache version number so when you startup, an existing
cache with a different version number will be deleted.	It works that way going
backwards too.	This is a feature.

	gordon Assignee
	Comment 22 • 22 years ago

This patch updates the build problem Doug found, the deadlock problem Darin
found, and adds truncation for the cache block files when clearing the disk
cache.

	gordon Assignee
	Comment 23 • 22 years ago

Here's the latest, incorporating all changes from Darin's review.

	Darin Fisher
	Comment 24 • 22 years ago

Comment on attachment 93783 [details] [diff] [review]
polished patch with updates from darin's review

sr=darin (nice work!)

	Jaime Rodriguez, Jr.
	Comment 25 • 22 years ago

a perf improvement sounds like a good thing to have for the next release.
nominating for nsbeta1.

	Darin Fisher
	Comment 26 • 22 years ago

fwiw: i'm seeing about a 3% page-load improvement with this patch under win2k
against the cowtools page loader test (850MHz PII laptop w/ 256 MB RAM with a
"pretty fast" IDE drive).  i'd expect results to vary considerably from machine
to machine.

	Darin Fisher
	Comment 27 • 22 years ago

also of interest, i spent some time browsing with this patch in my tree.  i
downloaded 3669 documents taking up approximately 27.5 MB in the disk cache.  of
those, only 339 ended up in separate files.  that's roughly 1/10th the number of
separate files stored in the disk cache!!

	Doug Turner (:dougt)
	Comment 28 • 22 years ago

Comment on attachment 93783 [details] [diff] [review]
polished patch with updates from darin's review

I really hate having all of the platform specific code in the
nsDiskCache::Truncate().  This kind of thing probably should be supported in
nspr.

	Darin Fisher
	Comment 29 • 22 years ago

dougt: see bug 88341 (gordon says he's planning on fixing that bug when he gets
back from vacation).

	Darin Fisher
	Comment 30 • 22 years ago

fixed-on-trunk :-)

	Aleksey Nogin
	Comment 31 • 22 years ago

This commit have added a new "might be used uninitialized" warning (see also bug
59652) on brad TBox:

+netwerk/cache/src/nsDiskCacheEntry.cpp:106
+ `PRInt32 pad' might be used uninitialized in this function

Indeed, if size is > 16384, the pad will be uninitialized!

Should I file a new bug on this?

	Darin Fisher
	Comment 32 • 22 years ago

see bug 161417 ... i'll take care of fixing warning there.

	John Unruh
	Comment 33 • 22 years ago

Verified per comment #30.