You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2005/11/07 01:43:08 UTC

Re: [PATCH]: Increase size of FSFS dir cache

On Mon, Oct 31, 2005 at 12:45:48PM -0700, Garrett Rooney wrote:
> On 10/29/05, Daniel Berlin <db...@dberlin.org> wrote:
> 
> > 1. Anybody who doesn't have enough memory to hold 128 dirs of their repo
> > in memory is probably in trouble anyway.  Assuming 100k of info per dir,
> > that's only ..... 12.8 meg of memory, *if they hit all the dirs*, and
> > they had probably about 1000 files per dir (to generate 100k of info).
> > This seems reasonable to me.
> 
> This seems like a reasonable tradeoff for the speed gain, although

Is it reasonable? Adding 12.8 meg in absolute terms isn't a big deal.
Now do it for 100 concurrent sessions.

Oops.

Individual connection footprints are just one piece of the puzzle.
They *still* have to stay tiny so that a given server can service more
than three clients.

I've seen CVS take several hundred meg. Hey, no problem. The server
had 4 gig of memory. Hah! It wasn't so fun as the server went into
swap death when a half-dozen people tried to update their client.

(and yes, I see this has been committed, but I still think there is
 concern around adding to the per-connection footprint like this)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: Increase size of FSFS dir cache

Posted by Daniel Berlin <db...@dberlin.org>.
On Sun, 2005-11-06 at 23:00 -0500, Greg Hudson wrote:
> On Sun, 2005-11-06 at 22:15 -0500, Daniel Berlin wrote:
> > 2. svn_stream_readline to read the hash tables for directories and
> > revprops, again and again and again and again (still, because every
> > svnserve instance it).  On gcc.gnu.org, this is actually cpu bound, but
> > still wastes about >20% of the cpu time per session, which is quite
> > high.  When it *does* hit the disk, it's ridiculously slow, because it
> > uses a 1 byte buffer.
> 
> The 1 byte buffer issue is a one-line fix, if you can figure out where
> the file is opened and add APR_BUFFERED.

Will do.


> > For 1.4, I think we need to do something about the serialized
> > hashtables.  Trivially, you could just prepend an encoded integer to
> > each line instead of using something that goes searching for newline.
> 
> I'm a little confused.  If I remember right, the format of a serialized
> hash table is a succession of:
> 
>   K <n>\n<n bytes of data>\n
>   V <m>\n<m bytes of data>\n
> 
> I might have picked a slightly different format, but I don't see how you
> can improve the intrinsic efficiency much.

Not intrinsicly, but without cross-file inlining, you get screwed here
by the call overhead to svn_stream read.  if you read them a ton of
times.

>  Punt the readline calls and
> read character-by-character, and you should get the same efficiency as
> you'd get with those prepended integers.
Possibly.

Anyway, i see there is actually some room for avoiding readline touching
a few bytes here.

We can make it avoid 3 more bytes than it does now to read the "V
<number>" and "K <number>" lines.  i'll see if it makes any real
difference.

BTW, another issue is that most of our revision files, when compressed
with SVNDIFF1, consist of large plaintext hashtables and lines of
changes :)  




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: Increase size of FSFS dir cache

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2005-11-06 at 22:15 -0500, Daniel Berlin wrote:
> 2. svn_stream_readline to read the hash tables for directories and
> revprops, again and again and again and again (still, because every
> svnserve instance it).  On gcc.gnu.org, this is actually cpu bound, but
> still wastes about >20% of the cpu time per session, which is quite
> high.  When it *does* hit the disk, it's ridiculously slow, because it
> uses a 1 byte buffer.

The 1 byte buffer issue is a one-line fix, if you can figure out where
the file is opened and add APR_BUFFERED.

> For 1.4, I think we need to do something about the serialized
> hashtables.  Trivially, you could just prepend an encoded integer to
> each line instead of using something that goes searching for newline.

I'm a little confused.  If I remember right, the format of a serialized
hash table is a succession of:

  K <n>\n<n bytes of data>\n
  V <m>\n<m bytes of data>\n

I might have picked a slightly different format, but I don't see how you
can improve the intrinsic efficiency much.  Punt the readline calls and
read character-by-character, and you should get the same efficiency as
you'd get with those prepended integers.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: Increase size of FSFS dir cache

Posted by Daniel Berlin <db...@dberlin.org>.
On Sun, 2005-11-06 at 19:51 -0800, Greg Stein wrote:
> On Sun, Nov 06, 2005 at 10:15:24PM -0500, Daniel Berlin wrote:
> >...
> > Would that satisfy you?
> 
> I wasn't ever *un*satisfied. I was responding to the notion of "trade
> off memory for speed" where nobody appeared to consider concurrency on
> the server which can cause a little bump to become overly large.
> 
> In this concrete case, yeah: not a big deal. As you point out, a lot
> of "ifs". But that 12 meg number did stick out :-)
> 
> >...
> > I believe we have more scalability issues you need to worry about with
> > serving 100 concurrent sessions than the size of the dirent cache.
> > Honestly, if you want to play the "oh, we need to serve 100 concurrent
> > sessions at once" game, here's some real data:
> 
> Fair enough :-) ... and yeah, I think it is a concern, when you look
> at orgs like the ASF and KDE which have big SVN repositories.
> 
> > oprofiling on the server with 20 concurrent sessions going shows that
> > most of the server time is being spent:
> > 
> > 1. md5'ing every single darn thing read from fsfs, every single time
> > it's read.  I've since hardcoded this off in our fsfs, instead having it
> > do it in svnadmin verify.  This was eating about 40% of the CPU time
> > spent per session on the server
> 
> Interesting. With a good policy of running verify, this is a decent
> tradeoff. Maybe for the admins who know they're doing a good job of
> verify, they could disable the checksums? It's a bit worrisome,
> though, because of all the potential failure modes that an admin could
> find themselves in (they aren't as smart as they thought, cron broke,
> forgot to verify that new repos, etc).

At least on gcc.gnu.org, we have rdiff backups for these directories
going back a while.

Plus offsite backups, which verify that no prior revision file ever
changes (IE that rsync doesn't transfer anything for them). :)

This actually does somewhat better than the checksums, because the
checksums only work when you read the revisions, so you'd need to be
vigilant about svnadmin verify anyway.

> 
> > 2. svn_stream_readline to read the hash tables for directories and
> > revprops, again and again and again and again (still, because every
> > svnserve instance it).  On gcc.gnu.org, this is actually cpu bound, but
> > still wastes about >20% of the cpu time per session, which is quite
> > high.  When it *does* hit the disk, it's ridiculously slow, because it
> > uses a 1 byte buffer.
> 
> Ouch. Time for that APR_UNBUFFERED patch, maybe? Or switch to a binary
> format for these instead? (e.g. len+value)
> 
> > If you want to get the memory footprint down, and get better
> > performance, make it so we don't *have* to cache so much data to get any
> > kind of performance.  Personally, i think it's somewhat funny that we
> > are spending most of the time processing revisions *hunting for newline
> > characters* in serialized hash tables
> 
> hehe... agreed :-)
> 
> >...
> > For 1.4, I think we need to do something about the serialized
> > hashtables.
> 
> Sure sounds that way. There would be a positive impact on the client,
> too, but concurrency doesn't really matter as much there :-)

Yeah.  In addition, A lot of gcc people want to use svk (because it's
common to have 5-20 branches and trees checked out), and they are
running up against the repo side issues like this, too :)

> 
> >...
> > Any of this will completely destroy any compatibility with the current
> > fsfs format, however.
> > Sadly, i'm not sure there *is* any way to keep compatibility while still
> > giving it the information to avoid svn_stream_readline.
> > 
> > (This was another reason i'm in favor of featurizing the fs.  We'd just
> > set "hash3" so that we knew how the hashtables should be stored)
> 
> Yup, it would break it, but I don't think that you can "featurize". An
> older SVN wouldn't know how to deal with the new/changed features, so
> it would be just as broken as if you rev'd the whole repository.

Yes, but once featurized, future svn's could keep that format without
too much trouble until you turn it off and enable "hash4" or "hash5".

It's more or less a way of specifying what has bumped, instead of just
"format" :)




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: Increase size of FSFS dir cache

Posted by Greg Stein <gs...@lyra.org>.
On Sun, Nov 06, 2005 at 10:15:24PM -0500, Daniel Berlin wrote:
>...
> Would that satisfy you?

I wasn't ever *un*satisfied. I was responding to the notion of "trade
off memory for speed" where nobody appeared to consider concurrency on
the server which can cause a little bump to become overly large.

In this concrete case, yeah: not a big deal. As you point out, a lot
of "ifs". But that 12 meg number did stick out :-)

>...
> I believe we have more scalability issues you need to worry about with
> serving 100 concurrent sessions than the size of the dirent cache.
> Honestly, if you want to play the "oh, we need to serve 100 concurrent
> sessions at once" game, here's some real data:

Fair enough :-) ... and yeah, I think it is a concern, when you look
at orgs like the ASF and KDE which have big SVN repositories.

> oprofiling on the server with 20 concurrent sessions going shows that
> most of the server time is being spent:
> 
> 1. md5'ing every single darn thing read from fsfs, every single time
> it's read.  I've since hardcoded this off in our fsfs, instead having it
> do it in svnadmin verify.  This was eating about 40% of the CPU time
> spent per session on the server

Interesting. With a good policy of running verify, this is a decent
tradeoff. Maybe for the admins who know they're doing a good job of
verify, they could disable the checksums? It's a bit worrisome,
though, because of all the potential failure modes that an admin could
find themselves in (they aren't as smart as they thought, cron broke,
forgot to verify that new repos, etc).

> 2. svn_stream_readline to read the hash tables for directories and
> revprops, again and again and again and again (still, because every
> svnserve instance it).  On gcc.gnu.org, this is actually cpu bound, but
> still wastes about >20% of the cpu time per session, which is quite
> high.  When it *does* hit the disk, it's ridiculously slow, because it
> uses a 1 byte buffer.

Ouch. Time for that APR_UNBUFFERED patch, maybe? Or switch to a binary
format for these instead? (e.g. len+value)

> If you want to get the memory footprint down, and get better
> performance, make it so we don't *have* to cache so much data to get any
> kind of performance.  Personally, i think it's somewhat funny that we
> are spending most of the time processing revisions *hunting for newline
> characters* in serialized hash tables

hehe... agreed :-)

>...
> For 1.4, I think we need to do something about the serialized
> hashtables.

Sure sounds that way. There would be a positive impact on the client,
too, but concurrency doesn't really matter as much there :-)

>...
> Any of this will completely destroy any compatibility with the current
> fsfs format, however.
> Sadly, i'm not sure there *is* any way to keep compatibility while still
> giving it the information to avoid svn_stream_readline.
> 
> (This was another reason i'm in favor of featurizing the fs.  We'd just
> set "hash3" so that we knew how the hashtables should be stored)

Yup, it would break it, but I don't think that you can "featurize". An
older SVN wouldn't know how to deal with the new/changed features, so
it would be just as broken as if you rev'd the whole repository.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: Increase size of FSFS dir cache

Posted by Daniel Berlin <db...@dberlin.org>.
On Sun, 2005-11-06 at 17:43 -0800, Greg Stein wrote:
> On Mon, Oct 31, 2005 at 12:45:48PM -0700, Garrett Rooney wrote:
> > On 10/29/05, Daniel Berlin <db...@dberlin.org> wrote:
> > 
> > > 1. Anybody who doesn't have enough memory to hold 128 dirs of their repo
> > > in memory is probably in trouble anyway.  Assuming 100k of info per dir,
> > > that's only ..... 12.8 meg of memory, *if they hit all the dirs*, and
> > > they had probably about 1000 files per dir (to generate 100k of info).
> > > This seems reasonable to me.
> > 
> > This seems like a reasonable tradeoff for the speed gain, although
> 
> Is it reasonable? Adding 12.8 meg in absolute terms isn't a big deal.
> Now do it for 100 concurrent sessions.
> 

If you look at the numbers above, you'll note this would take *100k* of
dirents per directory, and the hash being perfect, and them having at
least 128 dirs of 1000 files, and every concurrent session hitting every
single one of those directories, at the same time.

That's a lot of ifs.

> Oops.

Not so much.

The min footprint of svnserve on gcc.gnu.org was around 20 meg virt per
session, and 12 meg real before this change. So in a completely
ridiculous scenario, your footprint is now 30 meg virt and 24 meg real.

So before you needed 1.2 gig of memory, and now you need 2.4, again, in
this ridiculous scenarios.
IOW, it's not like i've increase the footprint from 3 meg to 2400 meg.

In real usage, all 20 concurrent sessions of svnserve going on
gcc.gnu.org right now are at .... 12.5 meg real. 

Thus, we could probably push the number down to 32 and still get the
same speedup, at least for gcc.  In the completely ridiculous scenario
above, that would produce an extra 3 meg footprint per client.  Would
that satisfy you?


> 
> Individual connection footprints are just one piece of the puzzle.
> They *still* have to stay tiny so that a given server can service more
> than three clients.

Of course. If the cost of serializing and unserializing dirents wasn't
(IE there was some magic self-contained format to them), i would have
just set up memcached, memcached this (because all these svnserves can
share the dirents), and been done with it.  However, i was doing this
because we were spending more time processing the directories again and
again than 

I believe we have more scalability issues you need to worry about with
serving 100 concurrent sessions than the size of the dirent cache.
Honestly, if you want to play the "oh, we need to serve 100 concurrent
sessions at once" game, here's some real data:

oprofiling on the server with 20 concurrent sessions going shows that
most of the server time is being spent:

1. md5'ing every single darn thing read from fsfs, every single time
it's read.  I've since hardcoded this off in our fsfs, instead having it
do it in svnadmin verify.  This was eating about 40% of the CPU time
spent per session on the server

2. svn_stream_readline to read the hash tables for directories and
revprops, again and again and again and again (still, because every
svnserve instance it).  On gcc.gnu.org, this is actually cpu bound, but
still wastes about >20% of the cpu time per session, which is quite
high.  When it *does* hit the disk, it's ridiculously slow, because it
uses a 1 byte buffer.

If you want to get the memory footprint down, and get better
performance, make it so we don't *have* to cache so much data to get any
kind of performance.  Personally, i think it's somewhat funny that we
are spending most of the time processing revisions *hunting for newline
characters* in serialized hash tables

This change was a last resort, trust me.
I spent a lot of time thinking about whether i could avoid these dir
reads in the first place, and hunting down callpaths to see if they
really needed to be doing what they were doing.

No dice.

For 1.4, I think we need to do something about the serialized
hashtables.  Trivially, you could just prepend an encoded integer to
each line instead of using something that goes searching for newline.
You could make up for the small increase n space usage by changing the
current serialization of lengths into variable size encoded ints instead
of written strings.  Or compressing the hashtables, which would also
help i/o for large dirs at a very small cpu time cost (zlib is quite
efficient for blocks of this size).
Any of this will completely destroy any compatibility with the current
fsfs format, however.
Sadly, i'm not sure there *is* any way to keep compatibility while still
giving it the information to avoid svn_stream_readline.

(This was another reason i'm in favor of featurizing the fs.  We'd just
set "hash3" so that we knew how the hashtables should be stored)


> I've seen CVS take several hundred meg. Hey, no problem. The server
> had 4 gig of memory. Hah! It wasn't so fun as the server went into
> swap death when a half-dozen people tried to update their client.

Without my change, it would have gone into i/o and cpu bound death
anyway long before.  Just FYI :)


> 
> (and yes, I see this has been committed, but I still think there is
>  concern around adding to the per-connection footprint like this)
> 
> Cheers,
> -g
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org