592520 - Do not fragment the hell out of CACHE__00[1-3]__

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Comment 1

[(dormant account)]

Attachment: proof of concept

This starts the cache files at 1mb. Then grows them by doubling the file size.

Comment 2

[(dormant account)]

Attachment: no frags

This works on Linux/Windows and likely on osx(will test on try)

Comment 3

[Benjamin Smedberg]

Comment on attachment 471214 [details] [diff] [review]
no frags

Mac doesn't have posix_fallocate, so the ifdef is definitely incorrect. It's not entirely clear to me whether we expect this method to be unimplemented in normal cases, and whether it's an advisory call or actually changes the file size in important ways. Can you explain (preferably in the doccomment).

Comment 4

[(dormant account)]

Attachment: no frags with a fallback for posix_fallocate

Added HAVE_POSIX_FALLOCATE(which turns out to be needed for equivalent functionality in sqlite http://mxr.mozilla.org/mozilla-central/source/db/sqlite3/src/sqlite3.c#25687) + fallback.

Comment 5

[(dormant account)]

Attachment: no frags with a fallback for posix_fallocate

This passes try.

Comment 6

[(dormant account)]

Found http://lists.apple.com/archives/darwin-dev/2007/Dec/msg00040.html which seems to show to how to do this on osx so I can avoid the glibc-style workaround and do this properly on all supported platforms.

Comment 7

[Benjamin Smedberg]

Comment on attachment 471714 [details] [diff] [review]
no frags with a fallback for posix_fallocate

r=me on the XPCOM bits. I did not review the necko bits.

Comment 8

[(dormant account)]

Comment on attachment 471714 [details] [diff] [review]
no frags with a fallback for posix_fallocate

Can you review the network bits? I'd like to land this asap. I found a profile where disk io is magnitudes slower than the network.

Comment 9

[(dormant account)]

Jason,
I've been waiting for a review for a week, can you give me an ETA? I'd like to land this asap.

Comment 10

[(dormant account)]

Attachment: magic mac fallocate dance

This is an addon patch that implement low-fragmentation allocation on mac.

Trivia: Mac is special in some ways, it allows one to demand a continuous chunk of disk... Which is cool. This allows one to also detect fragmentation. On the other hand ftruncate seems to actually write out zeros to disk instead of doing clever things on fs metadata level.

Comment 11

[Johnny Stenback (:jst)]

Blocking per discussion with Taras, high return fix here.

Comment 12

[Jason Duell]

Comment on attachment 471714 [details] [diff] [review]
no frags with a fallback for posix_fallocate

>+bool
>+nsDiskCacheBlockFile::Write(PRInt32 offset, const void *buf, PRInt32 amount)

>+        if (upTo > maxPreallocate) {
>+            mFileSize = upTo;
>+        } else {
>+            // Grow quickly between 1MB to 20MB
>+            if (mFileSize)
>+                while(mFileSize < upTo)
>+                    mFileSize *= 2;
>+        }

So we're doubling file size until we hit 20 MB, then just adding blocks incrementally?   It it worth doing continuing to do large fallocates after >20MB, esp on non-OSX?  (maybe not always doubling, but at least grabbing big fixed sized amounts).

Either way, +r.

Comment 13

[(dormant account)]

Attachment: no frags with a fallback for posix_fallocate/fcntl

Carrying over review from bsmedberg/jduel.

Comment 14

[Jason Duell]

Comment on attachment 475666 [details] [diff] [review]
no frags with a fallback for posix_fallocate/fcntl

This patch doesn't have the "start at 4mb, then grow by 4mb after we hit 20 mb" logic we talked about.  Another patch coming?

Comment 15

[(dormant account)]

Attachment: no frags with a fallback for posix_fallocate/fcntl(for real)

Comment 16

[Jason Duell]

Comment on attachment 475681 [details] [diff] [review]
no frags with a fallback for posix_fallocate/fcntl(for real)

So, just to be clear, this patch has the cache file grow in this series:  4, 8, 16, 20, 24, 28, ...

Almost doesn't seem worth the fuss to have the doubling logic in place when it's only used once.  How about 5 MB units, so 5, 10, 20, 25, 30, ...? 

I'm still not completely clear on the significance of the 20 MB filesize on OSX. What do we lose after files get bigger than that?

Comment 17

[(dormant account)]

(In reply to comment #16)
> Comment on attachment 475681 [details] [diff] [review]
> no frags with a fallback for posix_fallocate/fcntl(for real)
> 
> So, just to be clear, this patch has the cache file grow in this series:  4, 8,
> 16, 20, 24, 28, ...
> 
> Almost doesn't seem worth the fuss to have the doubling logic in place when
> it's only used once.  How about 5 MB units, so 5, 10, 20, 25, 30, ...? 
> 
> I'm still not completely clear on the significance of the 20 MB filesize on
> OSX. What do we lose after files get bigger than that?

Can get rid of the doubling logic if you like. So my growing approach minimizes fragmentation, but the file is still likely to be fragmented at least once per growth cycle. ie with your 5mb increment, there would be at least 4 fragments
for a 20mb file. So as long as the file is <20mb OSX will turn that into a single fragment.

Comment 18

[[e-mail]]

http://hg.mozilla.org/mozilla-central/rev/26e2971eeec9

Comment 19

[David Baron :dbaron:]

Attachment: fix non-libxul bustage

Comment 20

[(dormant account)]

nonlibxul bustage fix http://hg.mozilla.org/mozilla-central/rev/a9a27874f1c9