You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2004/10/08 19:19:13 UTC

[PATCH] Re: slow status

On Thu, 7 Oct 2004, Greg Hudson wrote:

> Presumably we're locking the anchor recursively.
>
> I thought we had fixed this ages ago, but I guess not.
>
Maybe the attached patch will fix this problem?

Re: [PATCH] Re: slow status

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 10 Oct 2004, Greg Hudson wrote:

> On Sun, 2004-10-10 at 12:04, Greg Hudson wrote:
> > I think doing that can also simplify the logic.  If there is a target,
> > lock the anchor non-recursively and then lock the target (recursively or
> > not, as specified).  If there is no target, lock the anchor recursively
> > or not as specified.  That's fewer cases than your patch has.
>
> That was imprecise and missed the subtlety that we need to lock the
> subdirs for a non-recursive status.  But there's still no need to
> consider more cases than I specified.  So, to be more precise:
>
>   If there is a separate anchor and target:
>     Lock the anchor with depth 0
>     Lock the target with depth (recursive ? -1 : 1)
>   Else
>     Lock the anchor with depth (recursive ? -1 : 1)
>
> No need to consider whether the target is a file or anything like that.
>
>
Hmmm... Seems like I have to check and only lock target if it is
non-empty *and* a directory. Locking a file in a locked directory gives an
wc locked error.

Thanks,
//Peter

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

Re: [PATCH] Re: slow status, next next version

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 25 Oct 2004, Barry Scott wrote:

> Am I right in assuming that this is still not fixed 1.1.1?
>
> I see 4.6 seconds to do svn st -N of the top of the 1.1.1 tag in my
> working copy.

> Let me ask a simple question: Why do you need to lock at all to get a
> status?
> If locking is a problem stop doing it :-)
>
Should be fixed in r11439, but it was fixed rather late in the 1.1.1 cycle
and I didn't want to include more stuff in 1.1.1 at that time. It owuld be
nice to know, however, if it makes any different if you could try the
patch out.

Regards,
//Peter

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

Re: [PATCH] Re: slow status, next next version

Posted by Barry Scott <ba...@barrys-emacs.org>.
Am I right in assuming that this is still not fixed 1.1.1?

I see 4.6 seconds to do svn st -N of the top of the 1.1.1 tag in my 
working copy.
After its in the windows cache its faster.

Let me ask a simple question: Why do you need to lock at all to get a 
status?
If locking is a problem stop doing it :-)

Barry


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

Re: [PATCH] Re: slow status, next next version

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Tue, 12 Oct 2004, Peter N. Lundblad wrote:

> On Tue, 12 Oct 2004, Peter N. Lundblad wrote:
>
> > Need to take are of the update case further down in the function as well.
> > Here's another try. Passes tests over ra_local. It wasn't as simple as you
> > stated, but I hope it is correct now.
> >
> And of course I hit send a little too early. It breaks a test. Will send
> another one. Sorry.
>
Here we go again. It actually passes the tests this time. I replaced a
wc_adm_retrieve with wc_adm_probe_retrieve...

Re: [PATCH] Re: slow status, next version

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Tue, 12 Oct 2004, Peter N. Lundblad wrote:

> Need to take are of the update case further down in the function as well.
> Here's another try. Passes tests over ra_local. It wasn't as simple as you
> stated, but I hope it is correct now.
>
> Thanks for this and many other reviews, Greg.
>
And of course I hit send a little too early. It breaks a test. Will send
another one. Sorry.

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

Re: [PATCH] Re: slow status, next version

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 10 Oct 2004, Greg Hudson wrote:

>   If there is a separate anchor and target:
>     Lock the anchor with depth 0
>     Lock the target with depth (recursive ? -1 : 1)
>   Else
>     Lock the anchor with depth (recursive ? -1 : 1)
>
Need to take are of the update case further down in the function as well.
Here's another try. Passes tests over ra_local. It wasn't as simple as you
stated, but I hope it is correct now.

Thanks for this and many other reviews, Greg.

Regards,
//Peter

Re: [PATCH] Re: slow status

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-10-10 at 12:04, Greg Hudson wrote:
> I think doing that can also simplify the logic.  If there is a target,
> lock the anchor non-recursively and then lock the target (recursively or
> not, as specified).  If there is no target, lock the anchor recursively
> or not as specified.  That's fewer cases than your patch has.

That was imprecise and missed the subtlety that we need to lock the
subdirs for a non-recursive status.  But there's still no need to
consider more cases than I specified.  So, to be more precise:

  If there is a separate anchor and target:
    Lock the anchor with depth 0
    Lock the target with depth (recursive ? -1 : 1)
  Else
    Lock the anchor with depth (recursive ? -1 : 1)

No need to consider whether the target is a file or anything like that.


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

Re: [PATCH] Re: slow status

Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2004-10-08 at 15:19, Peter N. Lundblad wrote:
> On Thu, 7 Oct 2004, Greg Hudson wrote:
> 
> > Presumably we're locking the anchor recursively.
> >
> > I thought we had fixed this ages ago, but I guess not.
> >
> Maybe the attached patch will fix this problem?

Locking the anchor with depth 2 is a botch.  If you look at
svn_client_switch(), you can see an example of locking the anchor
non-recursively and then locking the target.

I think doing that can also simplify the logic.  If there is a target,
lock the anchor non-recursively and then lock the target (recursively or
not, as specified).  If there is no target, lock the anchor recursively
or not as specified.  That's fewer cases than your patch has.


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