You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Malcolm Rowe <ma...@farside.org.uk> on 2007/10/11 09:33:17 UTC

Re: svn commit: r27097 - trunk/subversion/libsvn_fs_fs

On Wed, Oct 10, 2007 at 04:02:37PM -0700, glasser@tigris.org wrote:
> Move the cache of *immutable* DAG noderevs from revision roots to the
> svn_fs_t object.  This extends the lifetime of cache entries; in my
> test, a somewhat large diff operation on a repository in NFS got a 4x
> speedup.  Transaction DAG noderevs are still in the roots (though
> perhaps this should change).
> 

Nice change!  Is it worth considering whether this cache can go into the
per-filesystem object (fs_fs_shared_data_t) instead of the
per-filesystem-handle object (fs_fs_data_t)?

Obviously you'd have to add some locking, but it would help to ensure
that we can still benefit from the cache when we open a new handle.

Regards,
Malcolm

Re: svn commit: r27097 - trunk/subversion/libsvn_fs_fs

Posted by David Glasser <gl...@davidglasser.net>.
On 11/6/07, David Glasser <gl...@davidglasser.net> wrote:
> On 10/11/07, Malcolm Rowe <ma...@farside.org.uk> wrote:
> > On Wed, Oct 10, 2007 at 04:02:37PM -0700, glasser@tigris.org wrote:
> > > Move the cache of *immutable* DAG noderevs from revision roots to the
> > > svn_fs_t object.  This extends the lifetime of cache entries; in my
> > > test, a somewhat large diff operation on a repository in NFS got a 4x
> > > speedup.  Transaction DAG noderevs are still in the roots (though
> > > perhaps this should change).
> > >
> >
> > Nice change!  Is it worth considering whether this cache can go into the
> > per-filesystem object (fs_fs_shared_data_t) instead of the
> > per-filesystem-handle object (fs_fs_data_t)?
> >
> > Obviously you'd have to add some locking, but it would help to ensure
> > that we can still benefit from the cache when we open a new handle.
>
> That would work, but as you mentioned would require mutex overhead for
> threaded builds; one would need to benchmark multithreaded servers to
> see if it ends up being a win or a loss.

I added a comment noting the possibility in r27638.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

Re: svn commit: r27097 - trunk/subversion/libsvn_fs_fs

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Tue, Nov 06, 2007 at 04:09:11PM -0800, David Glasser wrote:
> On 10/11/07, Malcolm Rowe <ma...@farside.org.uk> wrote:
> > On Wed, Oct 10, 2007 at 04:02:37PM -0700, glasser@tigris.org wrote:
> > > Move the cache of *immutable* DAG noderevs from revision roots to the
> > > svn_fs_t object.  This extends the lifetime of cache entries; in my
> > > test, a somewhat large diff operation on a repository in NFS got a 4x
> > > speedup.  Transaction DAG noderevs are still in the roots (though
> > > perhaps this should change).
> > >
> >
> > Nice change!  Is it worth considering whether this cache can go into the
> > per-filesystem object (fs_fs_shared_data_t) instead of the
> > per-filesystem-handle object (fs_fs_data_t)?
> >
> > Obviously you'd have to add some locking, but it would help to ensure
> > that we can still benefit from the cache when we open a new handle.
> 
> That would work, but as you mentioned would require mutex overhead for
> threaded builds; one would need to benchmark multithreaded servers to
> see if it ends up being a win or a loss.
> 

Sure.  The reason I asked is that I didn't think we tended to keep hold
of svn_fs_t objects for very long - say one operation - so a cache that
persists beyond that seems like a good idea.

Regards,
Malcolm

Re: svn commit: r27097 - trunk/subversion/libsvn_fs_fs

Posted by David Glasser <gl...@davidglasser.net>.
On 10/11/07, Malcolm Rowe <ma...@farside.org.uk> wrote:
> On Wed, Oct 10, 2007 at 04:02:37PM -0700, glasser@tigris.org wrote:
> > Move the cache of *immutable* DAG noderevs from revision roots to the
> > svn_fs_t object.  This extends the lifetime of cache entries; in my
> > test, a somewhat large diff operation on a repository in NFS got a 4x
> > speedup.  Transaction DAG noderevs are still in the roots (though
> > perhaps this should change).
> >
>
> Nice change!  Is it worth considering whether this cache can go into the
> per-filesystem object (fs_fs_shared_data_t) instead of the
> per-filesystem-handle object (fs_fs_data_t)?
>
> Obviously you'd have to add some locking, but it would help to ensure
> that we can still benefit from the cache when we open a new handle.

That would work, but as you mentioned would require mutex overhead for
threaded builds; one would need to benchmark multithreaded servers to
see if it ends up being a win or a loss.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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