You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@hyrumwright.org> on 2011/05/16 17:35:28 UTC

Re: svn commit: r1103765 - in /subversion/trunk/subversion: libsvn_wc/update_editor.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/db-test.c tests/libsvn_wc/op-depth-test.c

On Mon, May 16, 2011 at 3:24 PM,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Mon May 16 15:24:59 2011
> New Revision: 1103765
>
> URL: http://svn.apache.org/viewvc?rev=1103765&view=rev
> Log:
> Finding the w/c base folder using svn_wc__db_wcroot_parse_local_abspath
> can be expensive if being called for many different paths (e.g. files instead
> of their parents whose w/c root information already got cached).
>
> Please note that this may not reduce the total number of stat calls, yet,
> as later invocations won't provide an suitable wri_abspath. However, Bert
> sees that as the basis to future improvements.

Let me see if I understand this (at a high level).  Instead of needing
to fetch the wcroot every time, we provide an already-fetched value to
avoid having to stat the local_abspath.

I'm not quite sure how I feel about this, personally.  I understand
the need to continue to improve performance, but the additional
conditionals and baggage resulting in carrying around another value
and optionally using it feel like it will just muddle the code.
Rather than require callers to caching and provide this value, it
seems that a more sensible option would be to implement the cache
internally.

I don't really know what to do about it right now, other than observe
that one of the major goals in wc-ng was to make it easier to develop
with.  If we're just going to start making spaghetti anew, that effort
is for not.

-Hyrum

Re: svn commit: r1103765 - in /subversion/trunk/subversion: libsvn_wc/update_editor.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/db-test.c tests/libsvn_wc/op-depth-test.c

Posted by Greg Stein <gs...@gmail.com>.
On May 16, 2011 11:36 AM, "Hyrum K Wright" <hy...@hyrumwright.org> wrote:
>
> On Mon, May 16, 2011 at 3:24 PM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Mon May 16 15:24:59 2011
> > New Revision: 1103765
> >
> > URL: http://svn.apache.org/viewvc?rev=1103765&view=rev
> > Log:
> > Finding the w/c base folder using svn_wc__db_wcroot_parse_local_abspath
> > can be expensive if being called for many different paths (e.g. files
instead
> > of their parents whose w/c root information already got cached).
> >
> > Please note that this may not reduce the total number of stat calls,
yet,
> > as later invocations won't provide an suitable wri_abspath. However,
Bert
> > sees that as the basis to future improvements.
>
> Let me see if I understand this (at a high level).  Instead of needing
> to fetch the wcroot every time, we provide an already-fetched value to
> avoid having to stat the local_abspath.
>
> I'm not quite sure how I feel about this, personally.  I understand
> the need to continue to improve performance, but the additional
> conditionals and baggage resulting in carrying around another value
> and optionally using it feel like it will just muddle the code.
> Rather than require callers to caching and provide this value, it
> seems that a more sensible option would be to implement the cache
> internally.
>
> I don't really know what to do about it right now, other than observe
> that one of the major goals in wc-ng was to make it easier to develop
> with.  If we're just going to start making spaghetti anew, that effort
> is for not.

Agreed.

Tge wri_abspath concept is for when you don't have another indicator of the
wcroot you're talking about. We should have one or the other, but not both.

Cheers,
-g

Re: svn commit: r1103765 - in /subversion/trunk/subversion: libsvn_wc/update_editor.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/db-test.c tests/libsvn_wc/op-depth-test.c

Posted by Greg Stein <gs...@gmail.com>.
On May 16, 2011 11:36 AM, "Hyrum K Wright" <hy...@hyrumwright.org> wrote:
>
> On Mon, May 16, 2011 at 3:24 PM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Mon May 16 15:24:59 2011
> > New Revision: 1103765
> >
> > URL: http://svn.apache.org/viewvc?rev=1103765&view=rev
> > Log:
> > Finding the w/c base folder using svn_wc__db_wcroot_parse_local_abspath
> > can be expensive if being called for many different paths (e.g. files
instead
> > of their parents whose w/c root information already got cached).
> >
> > Please note that this may not reduce the total number of stat calls,
yet,
> > as later invocations won't provide an suitable wri_abspath. However,
Bert
> > sees that as the basis to future improvements.
>
> Let me see if I understand this (at a high level).  Instead of needing
> to fetch the wcroot every time, we provide an already-fetched value to
> avoid having to stat the local_abspath.
>
> I'm not quite sure how I feel about this, personally.  I understand
> the need to continue to improve performance, but the additional
> conditionals and baggage resulting in carrying around another value
> and optionally using it feel like it will just muddle the code.
> Rather than require callers to caching and provide this value, it
> seems that a more sensible option would be to implement the cache
> internally.
>
> I don't really know what to do about it right now, other than observe
> that one of the major goals in wc-ng was to make it easier to develop
> with.  If we're just going to start making spaghetti anew, that effort
> is for not.

Agreed.

Tge wri_abspath concept is for when you don't have another indicator of the
wcroot you're talking about. We should have one or the other, but not both.

Cheers,
-g