You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2010/06/16 19:28:33 UTC

status information

Hi,

Since the svn_wc_status3_t struct is now missing the entry member, a lot 
of information previously available from a status call is missing.
I can work around a lot of that information using other svn API calls, 
but there's at least two pieces of info I'd really have back in a status 
call:

working size : the size of the files returned by 'svn st'. Since a 
status call has to stat the files anyway, that information shouldn't be 
expensive to get.

svn:needs-lock : I know this is a property and hasn't really anything to 
do with the status of items. But it's a very important piece of 
information. In TSVN, we use this to show a special overlay icon so 
users can immediately see that they have to get a lock before trying to 
edit the file. The 'readonly' flag on such files isn't of much help here 
since most apps don't warn users about it until they try to save their 
changes.
If it's too expensive to get the info whether the property is set, 
having the info whether the readonly filesystem flag is set would also 
work, and that info could be fetched with the file stat call that I 
think has to be done anyway.

Please consider adding these info pieces to the svn_wc_status3_t struct.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Thu, Jun 17, 2010 at 3:35 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Stefan Küng wrote:
>> So for me, 1.7 doesn't look like an improvement at all, no matter how
>> great WC-NG will get. There aren't any new features in it (yet) which I
>> haven't already used in 1.6, but I have to drop more and more existing
>> features.
>>
>> And from your comments I get the impression that it will get worse (for
>> me and TSVN) as time goes by.
>
> Let's put an end to this right now, shall we?
>
> TortoiseSVN has been and continues to be a significant source of the
> overwhelming success that Subversion enjoys today, and I don't see that
> changing unless that change is forced.  It would be incredibly foolish for
> this community to make API changes that serve only to lessen TortoiseSVN's
> ability to remain a viable Subversion client on a critical platform.
>
> There is a solution to be found here.  We will find it, and we will find it
> regardless of silly nuances and arbitrarily nods toward "the way we've
> always done it".  If that way isn't the way of sane progress, it's the wrong
> route for this project.

+1

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Küng wrote:
> So for me, 1.7 doesn't look like an improvement at all, no matter how
> great WC-NG will get. There aren't any new features in it (yet) which I
> haven't already used in 1.6, but I have to drop more and more existing
> features.
> 
> And from your comments I get the impression that it will get worse (for
> me and TSVN) as time goes by.

Let's put an end to this right now, shall we?

TortoiseSVN has been and continues to be a significant source of the
overwhelming success that Subversion enjoys today, and I don't see that
changing unless that change is forced.  It would be incredibly foolish for
this community to make API changes that serve only to lessen TortoiseSVN's
ability to remain a viable Subversion client on a critical platform.

There is a solution to be found here.  We will find it, and we will find it
regardless of silly nuances and arbitrarily nods toward "the way we've
always done it".  If that way isn't the way of sane progress, it's the wrong
route for this project.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 14:30, Stefan Sperling wrote:
> On Fri, Jun 18, 2010 at 01:19:18PM +0200, Stefan Küng wrote:
>> On 18.06.2010 06:46, Greg Stein wrote:
>>> The idea is: the status callback is invoked with the *limited* status
>>> information. If you want more, then from *within* that callback, make
>>> a function call back into the API to get the information you need.
>>>
>>> Look at that! No additional tree walks!
>>
>> Would work only if I'm allowed to use API's from libsvn_wc. But
>> those are supposed to go away.
>> In the callback, I can't use svn_client_* API's because the wc is
>> locked/in-use at that time and those API's will throw an error.
>
> The old APIs behaved like that with respect to locks.
>
> But the new ones can be different. Sqlite will manage reader locks for us.
> Any function call you make results in information being looked up in the
> database, except for information that can only be found on disk.
> That's my impression, anyway (someone please correct me if I'm wrong).
>
> The idea is not to remove the functionality libsvn_wc APIs provide,
> but to stop exposing the artificial barrier between libsvn_client
> and libsvn_wc in the public API. Instead, new libsvn_client APIs
> will provide any information you may need.

So as I said, what was suggested before simply can't be done in the 
current state. Thanks for finally getting a reasonable answer to my 
repeated complaints.

>>> Great. And if you were *listening*, Bert has said we'll do that at the
>>> client level. And even if we don't, then *you* can have a simple
>>> callback handler that grabs the extra information.
>>>
>>> This is not rocket science.
>>
>> Since we're not dealing with rockets here, I agree.
>
> :)
>
>> But please understand my situation here: you're talking about
>> removing APIs which I'm using extensively, and about removing the
>> APIs I would need to use in the callbacks. If you make the libsvn_wc
>> APIs non-public, then I can't do what you think I should do.
>
> We want to stop exposing the WC API so we can make changes more easily.
> A stable client API will be provided instead. I cannot tell you what
> this API will look like (that's what Bert and Greg are trying to get
> across), but I know that we won't remove functionality.

So again, I mentioned that I'm missing information which I can't get 
without severe performance hits. And I get smashed because of my concerns.
You're the first one to tell me that there will be new APIs that will 
allow me to do sometime later what I was yelled at to do now.

>>> And as Mike pointed out: we'd not going to screw you over. So stop
>>> your whining, make the extra calls in your callback, and let's move
>>> on. Offer some suggestions for improvements, rather than "wah wah, I
>>> don't like it".
>>
>> I made a perfectly fine suggestion, which got rejected only because
>> it's "against Subversion policies" and my question about where such
>> a policy is written down wasn't answered.
>
> I don't know either what Bert meant (maybe something related to
> binary compatibility?). It would be great to hear a better explanation.

Agreed. And I still think it would be good to have such a 'mask' 
parameter and have the svn lib do all the work: this prevents having all 
clients do the same work in (most likely) different ways and therefore 
get inconsistent behavior over the different clients.

>>> Simple fact is: we're going for *fast*. If you want something less
>>
>> Yes, that's what I hear every time I bring something up. But making
>> something fast is not the same as reducing the functionality.
>
> Any reduction in functionality will be temporary. It simply doesn't
> make any sense for us to remove functionality.
>
>> You could speed up a status call even more if you only return one
>> information: it's versioned/not versioned.
>> That would be very very fast but also useless for most.
>> Making something fast IMHO means making fast what's slow, not
>> crippling it until nobody can use it anymore.
>
> Again, I think this is not what the wc-ng people are doing,
> and you are being offensive by repeatedly claiming that this was
> the case. Please stop assuming that they want to work against you.

I better not comment on that...

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Sperling wrote:
> On Fri, Jun 18, 2010 at 01:19:18PM +0200, Stefan Küng wrote:
>> On 18.06.2010 06:46, Greg Stein wrote:
>>> The idea is: the status callback is invoked with the *limited* status
>>> information. If you want more, then from *within* that callback, make
>>> a function call back into the API to get the information you need.
>>>
>>> Look at that! No additional tree walks!
>> Would work only if I'm allowed to use API's from libsvn_wc. But
>> those are supposed to go away.
>> In the callback, I can't use svn_client_* API's because the wc is
>> locked/in-use at that time and those API's will throw an error.
> 
> The old APIs behaved like that with respect to locks.
> 
> But the new ones can be different. Sqlite will manage reader locks for us.
> Any function call you make results in information being looked up in the
> database, except for information that can only be found on disk.
> That's my impression, anyway (someone please correct me if I'm wrong).
> 
> The idea is not to remove the functionality libsvn_wc APIs provide,
> but to stop exposing the artificial barrier between libsvn_client
> and libsvn_wc in the public API. Instead, new libsvn_client APIs
> will provide any information you may need.

This is a critical point that I failed to mention in my previous "let's stop
adding to the svn-wc API" remark.  My desire is not to reveal less
information to third-parties -- it's to give them what they need from the
layer developed just for them (the *client* layer) while hiding the
implementation details of our WC.  Sorry if I've added to the confusion and
concern by not laying this plan out in full and only talking about half of it.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 14:30, Stefan Sperling wrote:
> On Fri, Jun 18, 2010 at 01:19:18PM +0200, Stefan Küng wrote:
>> On 18.06.2010 06:46, Greg Stein wrote:
>>> The idea is: the status callback is invoked with the *limited* status
>>> information. If you want more, then from *within* that callback, make
>>> a function call back into the API to get the information you need.
>>>
>>> Look at that! No additional tree walks!
>>
>> Would work only if I'm allowed to use API's from libsvn_wc. But
>> those are supposed to go away.
>> In the callback, I can't use svn_client_* API's because the wc is
>> locked/in-use at that time and those API's will throw an error.
>
> The old APIs behaved like that with respect to locks.
>
> But the new ones can be different. Sqlite will manage reader locks for us.
> Any function call you make results in information being looked up in the
> database, except for information that can only be found on disk.
> That's my impression, anyway (someone please correct me if I'm wrong).
>
> The idea is not to remove the functionality libsvn_wc APIs provide,
> but to stop exposing the artificial barrier between libsvn_client
> and libsvn_wc in the public API. Instead, new libsvn_client APIs
> will provide any information you may need.

So as I said, what was suggested before simply can't be done in the 
current state. Thanks for finally getting a reasonable answer to my 
repeated complaints.

>>> Great. And if you were *listening*, Bert has said we'll do that at the
>>> client level. And even if we don't, then *you* can have a simple
>>> callback handler that grabs the extra information.
>>>
>>> This is not rocket science.
>>
>> Since we're not dealing with rockets here, I agree.
>
> :)
>
>> But please understand my situation here: you're talking about
>> removing APIs which I'm using extensively, and about removing the
>> APIs I would need to use in the callbacks. If you make the libsvn_wc
>> APIs non-public, then I can't do what you think I should do.
>
> We want to stop exposing the WC API so we can make changes more easily.
> A stable client API will be provided instead. I cannot tell you what
> this API will look like (that's what Bert and Greg are trying to get
> across), but I know that we won't remove functionality.

So again, I mentioned that I'm missing information which I can't get 
without severe performance hits. And I get smashed because of my concerns.
You're the first one to tell me that there will be new APIs that will 
allow me to do sometime later what I was yelled at to do now.

>>> And as Mike pointed out: we'd not going to screw you over. So stop
>>> your whining, make the extra calls in your callback, and let's move
>>> on. Offer some suggestions for improvements, rather than "wah wah, I
>>> don't like it".
>>
>> I made a perfectly fine suggestion, which got rejected only because
>> it's "against Subversion policies" and my question about where such
>> a policy is written down wasn't answered.
>
> I don't know either what Bert meant (maybe something related to
> binary compatibility?). It would be great to hear a better explanation.

Agreed. And I still think it would be good to have such a 'mask' 
parameter and have the svn lib do all the work: this prevents having all 
clients do the same work in (most likely) different ways and therefore 
get inconsistent behavior over the different clients.

>>> Simple fact is: we're going for *fast*. If you want something less
>>
>> Yes, that's what I hear every time I bring something up. But making
>> something fast is not the same as reducing the functionality.
>
> Any reduction in functionality will be temporary. It simply doesn't
> make any sense for us to remove functionality.
>
>> You could speed up a status call even more if you only return one
>> information: it's versioned/not versioned.
>> That would be very very fast but also useless for most.
>> Making something fast IMHO means making fast what's slow, not
>> crippling it until nobody can use it anymore.
>
> Again, I think this is not what the wc-ng people are doing,
> and you are being offensive by repeatedly claiming that this was
> the case. Please stop assuming that they want to work against you.

I better not comment on that...

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 01:19:18PM +0200, Stefan Küng wrote:
> On 18.06.2010 06:46, Greg Stein wrote:
> >The idea is: the status callback is invoked with the *limited* status
> >information. If you want more, then from *within* that callback, make
> >a function call back into the API to get the information you need.
> >
> >Look at that! No additional tree walks!
> 
> Would work only if I'm allowed to use API's from libsvn_wc. But
> those are supposed to go away.
> In the callback, I can't use svn_client_* API's because the wc is
> locked/in-use at that time and those API's will throw an error.

The old APIs behaved like that with respect to locks.

But the new ones can be different. Sqlite will manage reader locks for us.
Any function call you make results in information being looked up in the
database, except for information that can only be found on disk.
That's my impression, anyway (someone please correct me if I'm wrong).

The idea is not to remove the functionality libsvn_wc APIs provide,
but to stop exposing the artificial barrier between libsvn_client
and libsvn_wc in the public API. Instead, new libsvn_client APIs
will provide any information you may need.

> >Great. And if you were *listening*, Bert has said we'll do that at the
> >client level. And even if we don't, then *you* can have a simple
> >callback handler that grabs the extra information.
> >
> >This is not rocket science.
> 
> Since we're not dealing with rockets here, I agree.

:)

> But please understand my situation here: you're talking about
> removing APIs which I'm using extensively, and about removing the
> APIs I would need to use in the callbacks. If you make the libsvn_wc
> APIs non-public, then I can't do what you think I should do.

We want to stop exposing the WC API so we can make changes more easily.
A stable client API will be provided instead. I cannot tell you what
this API will look like (that's what Bert and Greg are trying to get
across), but I know that we won't remove functionality.
 
> >And as Mike pointed out: we'd not going to screw you over. So stop
> >your whining, make the extra calls in your callback, and let's move
> >on. Offer some suggestions for improvements, rather than "wah wah, I
> >don't like it".
> 
> I made a perfectly fine suggestion, which got rejected only because
> it's "against Subversion policies" and my question about where such
> a policy is written down wasn't answered.

I don't know either what Bert meant (maybe something related to
binary compatibility?). It would be great to hear a better explanation.

> >Simple fact is: we're going for *fast*. If you want something less
> 
> Yes, that's what I hear every time I bring something up. But making
> something fast is not the same as reducing the functionality.

Any reduction in functionality will be temporary. It simply doesn't
make any sense for us to remove functionality.

> You could speed up a status call even more if you only return one
> information: it's versioned/not versioned.
> That would be very very fast but also useless for most.
> Making something fast IMHO means making fast what's slow, not
> crippling it until nobody can use it anymore.

Again, I think this is not what the wc-ng people are doing,
and you are being offensive by repeatedly claiming that this was
the case. Please stop assuming that they want to work against you.

> Also, you make it fast by removing information returned. Meaning I
> have to get that information myself. Will it still be faster then?

Yes. Because most of it is read from an sqlite DB, rather than from
files on disk which have to be parsed in their entirety before any
information can be returned. And the information needs to be cached
to make it fast! Try disabling entries-caching in 1.6.x and you will
see a massive performance decrease (and a couple of bugs where things
only work because of stale caches...)

Peace,
Stefan

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 01:19:18PM +0200, Stefan Küng wrote:
> On 18.06.2010 06:46, Greg Stein wrote:
> >The idea is: the status callback is invoked with the *limited* status
> >information. If you want more, then from *within* that callback, make
> >a function call back into the API to get the information you need.
> >
> >Look at that! No additional tree walks!
> 
> Would work only if I'm allowed to use API's from libsvn_wc. But
> those are supposed to go away.
> In the callback, I can't use svn_client_* API's because the wc is
> locked/in-use at that time and those API's will throw an error.

The old APIs behaved like that with respect to locks.

But the new ones can be different. Sqlite will manage reader locks for us.
Any function call you make results in information being looked up in the
database, except for information that can only be found on disk.
That's my impression, anyway (someone please correct me if I'm wrong).

The idea is not to remove the functionality libsvn_wc APIs provide,
but to stop exposing the artificial barrier between libsvn_client
and libsvn_wc in the public API. Instead, new libsvn_client APIs
will provide any information you may need.

> >Great. And if you were *listening*, Bert has said we'll do that at the
> >client level. And even if we don't, then *you* can have a simple
> >callback handler that grabs the extra information.
> >
> >This is not rocket science.
> 
> Since we're not dealing with rockets here, I agree.

:)

> But please understand my situation here: you're talking about
> removing APIs which I'm using extensively, and about removing the
> APIs I would need to use in the callbacks. If you make the libsvn_wc
> APIs non-public, then I can't do what you think I should do.

We want to stop exposing the WC API so we can make changes more easily.
A stable client API will be provided instead. I cannot tell you what
this API will look like (that's what Bert and Greg are trying to get
across), but I know that we won't remove functionality.
 
> >And as Mike pointed out: we'd not going to screw you over. So stop
> >your whining, make the extra calls in your callback, and let's move
> >on. Offer some suggestions for improvements, rather than "wah wah, I
> >don't like it".
> 
> I made a perfectly fine suggestion, which got rejected only because
> it's "against Subversion policies" and my question about where such
> a policy is written down wasn't answered.

I don't know either what Bert meant (maybe something related to
binary compatibility?). It would be great to hear a better explanation.

> >Simple fact is: we're going for *fast*. If you want something less
> 
> Yes, that's what I hear every time I bring something up. But making
> something fast is not the same as reducing the functionality.

Any reduction in functionality will be temporary. It simply doesn't
make any sense for us to remove functionality.

> You could speed up a status call even more if you only return one
> information: it's versioned/not versioned.
> That would be very very fast but also useless for most.
> Making something fast IMHO means making fast what's slow, not
> crippling it until nobody can use it anymore.

Again, I think this is not what the wc-ng people are doing,
and you are being offensive by repeatedly claiming that this was
the case. Please stop assuming that they want to work against you.

> Also, you make it fast by removing information returned. Meaning I
> have to get that information myself. Will it still be faster then?

Yes. Because most of it is read from an sqlite DB, rather than from
files on disk which have to be parsed in their entirety before any
information can be returned. And the information needs to be cached
to make it fast! Try disabling entries-caching in 1.6.x and you will
see a massive performance decrease (and a couple of bugs where things
only work because of stale caches...)

Peace,
Stefan

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 06:46, Greg Stein wrote:
> On Thu, Jun 17, 2010 at 16:48, Stefan Küng<to...@gmail.com>  wrote:
>> On 17.06.2010 22:35, Bert Huijben wrote:
>>>> Sure, I said before I can get any information myself, but only with
>>>> a severe performance hit. Because every time I call an svn_client_*
>>>> API, it walks the whole tree again, opens the files again, opens
>>>> the db again and again, ...
>>>
>>> svn_client doesn't open the db again. (The handle is cached in the
>>> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
>>> on your call it doesn't traverse any other nodes then the one you
>>> specify.
>>
>> But I need that information for all entries in the tree. So I have to
>> traverse the whole tree again.
>> Tree walks are expensive.
>
> That is totally ridiculous. No. You do NOT have to walk the tree again.
>
> The idea is: the status callback is invoked with the *limited* status
> information. If you want more, then from *within* that callback, make
> a function call back into the API to get the information you need.
>
> Look at that! No additional tree walks!

Would work only if I'm allowed to use API's from libsvn_wc. But those 
are supposed to go away.
In the callback, I can't use svn_client_* API's because the wc is 
locked/in-use at that time and those API's will throw an error.

>> I don't have a problem calling another API to get additional info where i
>> need it only for one or just a few items. But most of the time, I need that
>> information for the whole tree.
>
> Great. And if you were *listening*, Bert has said we'll do that at the
> client level. And even if we don't, then *you* can have a simple
> callback handler that grabs the extra information.
>
> This is not rocket science.

Since we're not dealing with rockets here, I agree.
But please understand my situation here: you're talking about removing 
APIs which I'm using extensively, and about removing the APIs I would 
need to use in the callbacks. If you make the libsvn_wc APIs non-public, 
then I can't do what you think I should do.

> And as Mike pointed out: we'd not going to screw you over. So stop
> your whining, make the extra calls in your callback, and let's move
> on. Offer some suggestions for improvements, rather than "wah wah, I
> don't like it".

I made a perfectly fine suggestion, which got rejected only because it's 
"against Subversion policies" and my question about where such a policy 
is written down wasn't answered.

> Simple fact is: we're going for *fast*. If you want something less

Yes, that's what I hear every time I bring something up. But making 
something fast is not the same as reducing the functionality.
You could speed up a status call even more if you only return one 
information: it's versioned/not versioned.
That would be very very fast but also useless for most.
Making something fast IMHO means making fast what's slow, not crippling 
it until nobody can use it anymore.

Also, you make it fast by removing information returned. Meaning I have 
to get that information myself. Will it still be faster then? Or will 
all the additional API calls I have to make to get the same info as 
before add up and make everything much slower than before? In that case, 
it's not faster.

> fast, then you can do that on your own side of the callbacks. All the
> information is there, and a call into the APIs is no more expensive
> than what our old code used to do. When it gets down to it, the SAME
> function is used to do the "is modified?" check. So what the hell are
> you complaining about, if you have to make the call instead of the svn
> library? Not to mention, Bert saying that libsvn_client will do all
> that for you, anyways, in a consistent fashion.
>
> Stop the noise. Move along.

I'm sorry. I'm only trying to raise my concerns here.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 14:15, Stefan Sperling wrote:
> On Fri, Jun 18, 2010 at 02:13:16PM +0200, Stefan Sperling wrote:
>> We've discussed this before, so I think you're aware of benchmarks
>> reports such as http://svn.haxx.se/dev/archive-2010-02/0241.shtml
>
> Another interesting one is here:
> http://svn.haxx.se/dev/archive-2010-02/0457.shtml

Yes, I know about those. I actually have those bookmarked.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 02:13:16PM +0200, Stefan Sperling wrote:
> We've discussed this before, so I think you're aware of benchmarks
> reports such as http://svn.haxx.se/dev/archive-2010-02/0241.shtml

Another interesting one is here:
http://svn.haxx.se/dev/archive-2010-02/0457.shtml

Stefan

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 01:30:59PM +0200, Stefan Küng wrote:
> Also, the speed hasn't improved much in the last 8 months. It's
> still 11 times (!!) slower than 1.6.x. Since there was some talk
> about having 1.7 finished for devs to test it this fall - well, I
> think you can guess my concerns here.

I hear your concerns about timeline. Fine. WC-NG is taking time.
The only solution to that is throwing more people at the problem.
It can still handle a couple more developers before the mythical
man month kicks in.

Just because it hasn't gotten any faster yet does not mean
it won't get any faster. We're not at single-db yet.
We've discussed this before, so I think you're aware of benchmarks
reports such as http://svn.haxx.se/dev/archive-2010-02/0241.shtml

Stefan

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 01:30:59PM +0200, Stefan Küng wrote:
> Also, the speed hasn't improved much in the last 8 months. It's
> still 11 times (!!) slower than 1.6.x. Since there was some talk
> about having 1.7 finished for devs to test it this fall - well, I
> think you can guess my concerns here.

I hear your concerns about timeline. Fine. WC-NG is taking time.
The only solution to that is throwing more people at the problem.
It can still handle a couple more developers before the mythical
man month kicks in.

Just because it hasn't gotten any faster yet does not mean
it won't get any faster. We're not at single-db yet.
We've discussed this before, so I think you're aware of benchmarks
reports such as http://svn.haxx.se/dev/archive-2010-02/0241.shtml

Stefan

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 11:15, Stefan Sperling wrote:

> There is a background story here.
>
> Stefan Küng has repeatedly been complaining about wc-ng.

Not complaining about wc-ng, but about the performance of it. That's a 
big difference.

> His perception seems to be that wc-ng will ultimately hurt TortoiseSVN.
> But this is not the intention of the Subversion project (of which Stefan
> Küng is part of, BTW). Rather, the intention is to improve virtually
> everything in working copy land for *all* Subversion clients.
>
> I think Stefan Küng is expecting too much from wc-ng too early. The APIs
> and implementation are still in flux and not finished. It is good that he
> is voicing his concerns, because it allows the Subversion project to cater
> to TortoiseSVN's needs. He isn't aware of some radical changes going on,
> and naturally frowns upon them. But some of his comments come across as
> a bit too dismissive.

I'm perfectly aware of the radical changes that are going on. And that's 
what worries me. Every time I check what has changed I find more and 
more things getting worse. And every time I raise my concerns I'm 
promised that things will get better - and next time I check another 
(usually small) thing I'm depending on got missing/removed or changed in 
a way I can't use it anymore.

Also, the speed hasn't improved much in the last 8 months. It's still 11 
times (!!) slower than 1.6.x. Since there was some talk about having 1.7 
finished for devs to test it this fall - well, I think you can guess my 
concerns here.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 11:15, Stefan Sperling wrote:

> There is a background story here.
>
> Stefan Küng has repeatedly been complaining about wc-ng.

Not complaining about wc-ng, but about the performance of it. That's a 
big difference.

> His perception seems to be that wc-ng will ultimately hurt TortoiseSVN.
> But this is not the intention of the Subversion project (of which Stefan
> Küng is part of, BTW). Rather, the intention is to improve virtually
> everything in working copy land for *all* Subversion clients.
>
> I think Stefan Küng is expecting too much from wc-ng too early. The APIs
> and implementation are still in flux and not finished. It is good that he
> is voicing his concerns, because it allows the Subversion project to cater
> to TortoiseSVN's needs. He isn't aware of some radical changes going on,
> and naturally frowns upon them. But some of his comments come across as
> a bit too dismissive.

I'm perfectly aware of the radical changes that are going on. And that's 
what worries me. Every time I check what has changed I find more and 
more things getting worse. And every time I raise my concerns I'm 
promised that things will get better - and next time I check another 
(usually small) thing I'm depending on got missing/removed or changed in 
a way I can't use it anymore.

Also, the speed hasn't improved much in the last 8 months. It's still 11 
times (!!) slower than 1.6.x. Since there was some talk about having 1.7 
finished for devs to test it this fall - well, I think you can guess my 
concerns here.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 08:25:24AM +0100, Becker, Thomas wrote:
> Greg,
> 
> I'm an outsider to the Subversion development community so you may be surprised by me jumping into this discussion. But as you were shouting to the public you should expect this.
> 
> I'm shocked about the tone of your post and I don't see how calling someone "whining" can help resolve technical misunderstandings.

There is a background story here.

Stefan Küng has repeatedly been complaining about wc-ng.
His perception seems to be that wc-ng will ultimately hurt TortoiseSVN.
But this is not the intention of the Subversion project (of which Stefan
Küng is part of, BTW). Rather, the intention is to improve virtually
everything in working copy land for *all* Subversion clients.

I think Stefan Küng is expecting too much from wc-ng too early. The APIs
and implementation are still in flux and not finished. It is good that he
is voicing his concerns, because it allows the Subversion project to cater
to TortoiseSVN's needs. He isn't aware of some radical changes going on,
and naturally frowns upon them. But some of his comments come across as
a bit too dismissive.

I can understand Greg taking offence when Stefan says things like "1.7 doesn't
look like an improvement at all, no matter how great WC-NG will get",
because Greg is one of the primary drivers of the wc-ng effort and he
is putting a lot of effort and energy into it. Greg is trying to solve,
in his spare time, really difficult problems for Subversion (and ultimately
for TortoiseSVN). And yes, Greg is easy to wind up, but he also winds down
very quickly.
 
I hope this clears the air a bit.

Thanks for raising the issue, Thomas. It is good that people are
concerned about keeping communication within this community free
of unnecessary noise.

Stefan (Sperling)

Re: status information

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 18, 2010 at 08:25:24AM +0100, Becker, Thomas wrote:
> Greg,
> 
> I'm an outsider to the Subversion development community so you may be surprised by me jumping into this discussion. But as you were shouting to the public you should expect this.
> 
> I'm shocked about the tone of your post and I don't see how calling someone "whining" can help resolve technical misunderstandings.

There is a background story here.

Stefan Küng has repeatedly been complaining about wc-ng.
His perception seems to be that wc-ng will ultimately hurt TortoiseSVN.
But this is not the intention of the Subversion project (of which Stefan
Küng is part of, BTW). Rather, the intention is to improve virtually
everything in working copy land for *all* Subversion clients.

I think Stefan Küng is expecting too much from wc-ng too early. The APIs
and implementation are still in flux and not finished. It is good that he
is voicing his concerns, because it allows the Subversion project to cater
to TortoiseSVN's needs. He isn't aware of some radical changes going on,
and naturally frowns upon them. But some of his comments come across as
a bit too dismissive.

I can understand Greg taking offence when Stefan says things like "1.7 doesn't
look like an improvement at all, no matter how great WC-NG will get",
because Greg is one of the primary drivers of the wc-ng effort and he
is putting a lot of effort and energy into it. Greg is trying to solve,
in his spare time, really difficult problems for Subversion (and ultimately
for TortoiseSVN). And yes, Greg is easy to wind up, but he also winds down
very quickly.
 
I hope this clears the air a bit.

Thanks for raising the issue, Thomas. It is good that people are
concerned about keeping communication within this community free
of unnecessary noise.

Stefan (Sperling)

AW: status information

Posted by "Becker, Thomas" <Th...@torex.com>.
Greg,

I'm an outsider to the Subversion development community so you may be surprised by me jumping into this discussion. But as you were shouting to the public you should expect this.

I'm shocked about the tone of your post and I don't see how calling someone "whining" can help resolve technical misunderstandings.

Sorry for staining this thread with my emotional rant but there are two issues you should be aware of:

- all posts to dev@subversion are readable by the interested public and affect the reputation  of the Subversion developer community as it is recognized by outsiders,

- it takes quite some (emotional) effort for the attacked person to recover and come back to the professional level, effort that you drew away from work on the subject.

Thomas

--

Thomas Becker
Software Development, Torex
T: +49 (0)30 49901-0 E: thomas.becker@torex.com
Torex Retail Solutions GmbH, Salzufer 8, D-10587 Berlin
T: +49 (0)30 49901-0  F: +49 (0)30 49901-139  www.torex.de


> -----Ursprüngliche Nachricht-----
> Von: Greg Stein [mailto:gstein@gmail.com]
> Gesendet: Freitag, 18. Juni 2010 06:47
> An: Stefan Küng
> Cc: dev@subversion.apache.org
> Betreff: Re: status information
> 
> On Thu, Jun 17, 2010 at 16:48, Stefan Küng <to...@gmail.com> wrote:
> > On 17.06.2010 22:35, Bert Huijben wrote:
> >>> Sure, I said before I can get any information myself, but only with
> >>> a severe performance hit. Because every time I call an svn_client_*
> >>> API, it walks the whole tree again, opens the files again, opens
> >>> the db again and again, ...
> >>
> >> svn_client doesn't open the db again. (The handle is cached in the
> >> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
> >> on your call it doesn't traverse any other nodes then the one you
> >> specify.
> >
> > But I need that information for all entries in the tree. So I have to
> > traverse the whole tree again.
> > Tree walks are expensive.
> 
> That is totally ridiculous. No. You do NOT have to walk the tree again.
> 
> The idea is: the status callback is invoked with the *limited* status
> information. If you want more, then from *within* that callback, make
> a function call back into the API to get the information you need.
> 
> Look at that! No additional tree walks!
> 
> > I don't have a problem calling another API to get additional info where
> i
> > need it only for one or just a few items. But most of the time, I need
> that
> > information for the whole tree.
> 
> Great. And if you were *listening*, Bert has said we'll do that at the
> client level. And even if we don't, then *you* can have a simple
> callback handler that grabs the extra information.
> 
> This is not rocket science.
> 
> And as Mike pointed out: we'd not going to screw you over. So stop
> your whining, make the extra calls in your callback, and let's move
> on. Offer some suggestions for improvements, rather than "wah wah, I
> don't like it".
> 
> Simple fact is: we're going for *fast*. If you want something less
> fast, then you can do that on your own side of the callbacks. All the
> information is there, and a call into the APIs is no more expensive
> than what our old code used to do. When it gets down to it, the SAME
> function is used to do the "is modified?" check. So what the hell are
> you complaining about, if you have to make the call instead of the svn
> library? Not to mention, Bert saying that libsvn_client will do all
> that for you, anyways, in a consistent fashion.
> 
> Stop the noise. Move along.
> 
> -g

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 06:46, Greg Stein wrote:
> On Thu, Jun 17, 2010 at 16:48, Stefan Küng<to...@gmail.com>  wrote:
>> On 17.06.2010 22:35, Bert Huijben wrote:
>>>> Sure, I said before I can get any information myself, but only with
>>>> a severe performance hit. Because every time I call an svn_client_*
>>>> API, it walks the whole tree again, opens the files again, opens
>>>> the db again and again, ...
>>>
>>> svn_client doesn't open the db again. (The handle is cached in the
>>> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
>>> on your call it doesn't traverse any other nodes then the one you
>>> specify.
>>
>> But I need that information for all entries in the tree. So I have to
>> traverse the whole tree again.
>> Tree walks are expensive.
>
> That is totally ridiculous. No. You do NOT have to walk the tree again.
>
> The idea is: the status callback is invoked with the *limited* status
> information. If you want more, then from *within* that callback, make
> a function call back into the API to get the information you need.
>
> Look at that! No additional tree walks!

Would work only if I'm allowed to use API's from libsvn_wc. But those 
are supposed to go away.
In the callback, I can't use svn_client_* API's because the wc is 
locked/in-use at that time and those API's will throw an error.

>> I don't have a problem calling another API to get additional info where i
>> need it only for one or just a few items. But most of the time, I need that
>> information for the whole tree.
>
> Great. And if you were *listening*, Bert has said we'll do that at the
> client level. And even if we don't, then *you* can have a simple
> callback handler that grabs the extra information.
>
> This is not rocket science.

Since we're not dealing with rockets here, I agree.
But please understand my situation here: you're talking about removing 
APIs which I'm using extensively, and about removing the APIs I would 
need to use in the callbacks. If you make the libsvn_wc APIs non-public, 
then I can't do what you think I should do.

> And as Mike pointed out: we'd not going to screw you over. So stop
> your whining, make the extra calls in your callback, and let's move
> on. Offer some suggestions for improvements, rather than "wah wah, I
> don't like it".

I made a perfectly fine suggestion, which got rejected only because it's 
"against Subversion policies" and my question about where such a policy 
is written down wasn't answered.

> Simple fact is: we're going for *fast*. If you want something less

Yes, that's what I hear every time I bring something up. But making 
something fast is not the same as reducing the functionality.
You could speed up a status call even more if you only return one 
information: it's versioned/not versioned.
That would be very very fast but also useless for most.
Making something fast IMHO means making fast what's slow, not crippling 
it until nobody can use it anymore.

Also, you make it fast by removing information returned. Meaning I have 
to get that information myself. Will it still be faster then? Or will 
all the additional API calls I have to make to get the same info as 
before add up and make everything much slower than before? In that case, 
it's not faster.

> fast, then you can do that on your own side of the callbacks. All the
> information is there, and a call into the APIs is no more expensive
> than what our old code used to do. When it gets down to it, the SAME
> function is used to do the "is modified?" check. So what the hell are
> you complaining about, if you have to make the call instead of the svn
> library? Not to mention, Bert saying that libsvn_client will do all
> that for you, anyways, in a consistent fashion.
>
> Stop the noise. Move along.

I'm sorry. I'm only trying to raise my concerns here.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Jun 18, 2010 at 14:07, Mark Phippard <ma...@gmail.com> wrote:
>...
> I would like to also think that we will not even consider shipping
> 1.7.0 unless it is faster overall than 1.6, and that would likely
> include being faster for the API usage patterns of TortoiseSVN.

Absolutely true. Which is why aspersions against that are so freakin' annoying.

> A couple other points/questions.
>
> Stefan seems to be operating under the impression that the public
> libsvn_wc API is going away in 1.7.  I did not think that was true?  I

It is not true. libsvn_wc will continue to exist and be supported.
Clients may need to call into it, as far as I'm concerned.

Others would like clients to only use libsvn_client functions. *shrug*

We might not add public functions to the API, but we're keeping pretty
much all existing concepts there and available.

>...
> Second, Stefan's idea of an API with flags about what you want to
> retrieve makes sense to me IF we are talking about a client API.  This
> would seem like a good way to keep callers of the API writing to the
> client API and allow us to deal with using the callbacks mentioned
> from within the implementation of the client API based on the flags
> passed.  I do not get the push-back to this idea.

The functions in our API which take flags have proven to be the most
problematic over time, IMO. To hazard a guess, I think it is a cop-out
to thinking harder about the semantics of what the code is really
trying to do. So with the loss of those semantics, we end up with
less-maintainable code.

Cheers,
-g

Re: status information

Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jun 18, 2010 at 1:58 PM, C. Michael Pilato <cm...@collab.net> wrote:

> Stefan:  As I've said before, TortoiseSVN will have what it needs to
> function and function well.  The code is in a crazy state of flux right now,
> though, and we don't want to wind up whipping out un-thought-out APIs to
> satisfy your needs *today* that we'll have to live with forever.  Please
> allow us more time to get past the plumbing stages of the WC-NG transitional
> work before asking us to address your needs.  We really do want to give you
> the information you need; we further want to give it to you in a way that
> doesn't bind our hands in the future.

I would like to also think that we will not even consider shipping
1.7.0 unless it is faster overall than 1.6, and that would likely
include being faster for the API usage patterns of TortoiseSVN.

A couple other points/questions.

Stefan seems to be operating under the impression that the public
libsvn_wc API is going away in 1.7.  I did not think that was true?  I
know I have personally argued we should not be as worried about the
backwards compat. if we can make a better API, but I have not heard us
say we are not going to provide API.

Second, Stefan's idea of an API with flags about what you want to
retrieve makes sense to me IF we are talking about a client API.  This
would seem like a good way to keep callers of the API writing to the
client API and allow us to deal with using the callbacks mentioned
from within the implementation of the client API based on the flags
passed.  I do not get the push-back to this idea.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: status information

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Jun 18, 2010 at 13:58, C. Michael Pilato <cm...@collab.net> wrote:
>...
> Greg:  See above.  We've plenty of WC-NG plumbing to deal with before ever
> needing to engage the topic of "which of a handful of call and response
> paradigms will do the job and make us happiest".  Can we defer that decision
> until later?  Maybe a more mature WC-NG will help us to make better such
> decisions.

That's why I didn't engage in the first place. I didn't like it, and
didn't feel a need to respond *now*. Stefan pushed on it, so I
responded with my thoughts.

*shrug*

It's why I got so frustrated and slashed back so hard last night. I
think it is wrong to focus on performance problems and APIs today.
You're right: there is a ton to do. We know that, and we have plans
for the next N steps already. Until we complete those plans, then the
rest is moot. And *pushing* on us before we complete those plans is
annoying, to say the least.

Let us complete what we know is necessary, and THEN get a good look at
what is left to enable clients to function as efficiently as we
believe they can.

Cheers,
-g

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Stefan Küng wrote:
>> And I thought this mailing list was to be used to discuss stuff, raise
>> concerns and get to an agreement on how things should be done. Silly me.
>> Seems it's only "take what's there and stop complaining".
>>
>>
>> Your attitude sucks.
>>
>>
>> Stefan
>>
>> P.S. Since I have now a week off from work, I'll take the time to think
>> about whether I want to keep working on TSVN. From what I see on trunk,
>> what I get here about where 1.7 is going and the attitude it doesn't
>> seem worth my effort anymore. Seems TSVN 1.7 can only get worse than
>> 1.6, so what's the point.
>> Don't expect anymore posts from me, at least until I made my decision.
> 
> Guys, this is ridiculous.  Neither "I don't like it so Subversion won't do
> it that way" nor threats to take one's toys and go home are productive.  The
> non-technical bits of this thread have played out like a preschool sandbox;
> the technical bits haven't been allowed to play out at all because the
> atmosphere isn't conducive to decent discussion.  Are you kidding me?!

Sorry for self-replying, but I don't want to leave things on a negative note
myself.

Stefan:  As I've said before, TortoiseSVN will have what it needs to
function and function well.  The code is in a crazy state of flux right now,
though, and we don't want to wind up whipping out un-thought-out APIs to
satisfy your needs *today* that we'll have to live with forever.  Please
allow us more time to get past the plumbing stages of the WC-NG transitional
work before asking us to address your needs.  We really do want to give you
the information you need; we further want to give it to you in a way that
doesn't bind our hands in the future.

Greg:  See above.  We've plenty of WC-NG plumbing to deal with before ever
needing to engage the topic of "which of a handful of call and response
paradigms will do the job and make us happiest".  Can we defer that decision
until later?  Maybe a more mature WC-NG will help us to make better such
decisions.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Küng wrote:
> And I thought this mailing list was to be used to discuss stuff, raise
> concerns and get to an agreement on how things should be done. Silly me.
> Seems it's only "take what's there and stop complaining".
> 
> 
> Your attitude sucks.
> 
> 
> Stefan
> 
> P.S. Since I have now a week off from work, I'll take the time to think
> about whether I want to keep working on TSVN. From what I see on trunk,
> what I get here about where 1.7 is going and the attitude it doesn't
> seem worth my effort anymore. Seems TSVN 1.7 can only get worse than
> 1.6, so what's the point.
> Don't expect anymore posts from me, at least until I made my decision.

Guys, this is ridiculous.  Neither "I don't like it so Subversion won't do
it that way" nor threats to take one's toys and go home are productive.  The
non-technical bits of this thread have played out like a preschool sandbox;
the technical bits haven't been allowed to play out at all because the
atmosphere isn't conducive to decent discussion.  Are you kidding me?!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 19:11, Greg Stein wrote:
> On Fri, Jun 18, 2010 at 12:28, Stefan Küng<to...@gmail.com>  wrote:
>> ...
>> Since you obviously haven't read my post here:
>> http://svn.haxx.se/dev/archive-2010-06/0250.shtml
>
> I read it. I read practically every post on dev@ *AND* most of the
> posts to commits@.
>
> I didn't respond because I didn't like the idea. We've used flag-based
> APIs in the past (entry_modify comes to mind), and they all tend to
> suck in the long run. They tend to hide what the function is used for,
> and make it very hard to reason about the function. For example,
> searching thru the code, I find the following:
>
>    svn_wc__entry_modify(db, local_abspath, svn_kind_unknown,
> &tmp_entry, modify_flags, scratch_pool);
>
> That tells me nothing about what that call is doing. So I need to open
> an editor, look for the call, and track down all assignments to
> modify_flags. And I also need to ensure each member of tmp_entry
> denoted by modify_flags has been assigned.
>
> Just way too hard to work with, and so we're undoing all of those in libsvn_wc.

That's what a documentation of APIs and flags are usually for, so you 
don't have to track down the code.
But anyway, this isn't an argument against keeping the features, only 
about a mask param.
So how about passing a struct which has bools instead of a mask?
Something like this:

struct status_info_req_t
{
bool get_modifications;
bool get_needslock;
bool get_filesize;
bool get_whatever;
}

With this, you immediately can see in the code what each flag is used 
for and what it means.

> At this point, use libsvn_wc APIs if you need to get something done. I
> don't care if you need to use them. Others say "only client".
> Whatever. The simple fact is: the API isn't "done", so use whatever
> you need.

And I thought this mailing list was to be used to discuss stuff, raise 
concerns and get to an agreement on how things should be done. Silly me. 
Seems it's only "take what's there and stop complaining".


Your attitude sucks.


Stefan

P.S. Since I have now a week off from work, I'll take the time to think 
about whether I want to keep working on TSVN. From what I see on trunk, 
what I get here about where 1.7 is going and the attitude it doesn't 
seem worth my effort anymore. Seems TSVN 1.7 can only get worse than 
1.6, so what's the point.
Don't expect anymore posts from me, at least until I made my decision.

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 19:11, Greg Stein wrote:
> On Fri, Jun 18, 2010 at 12:28, Stefan Küng<to...@gmail.com>  wrote:
>> ...
>> Since you obviously haven't read my post here:
>> http://svn.haxx.se/dev/archive-2010-06/0250.shtml
>
> I read it. I read practically every post on dev@ *AND* most of the
> posts to commits@.
>
> I didn't respond because I didn't like the idea. We've used flag-based
> APIs in the past (entry_modify comes to mind), and they all tend to
> suck in the long run. They tend to hide what the function is used for,
> and make it very hard to reason about the function. For example,
> searching thru the code, I find the following:
>
>    svn_wc__entry_modify(db, local_abspath, svn_kind_unknown,
> &tmp_entry, modify_flags, scratch_pool);
>
> That tells me nothing about what that call is doing. So I need to open
> an editor, look for the call, and track down all assignments to
> modify_flags. And I also need to ensure each member of tmp_entry
> denoted by modify_flags has been assigned.
>
> Just way too hard to work with, and so we're undoing all of those in libsvn_wc.

That's what a documentation of APIs and flags are usually for, so you 
don't have to track down the code.
But anyway, this isn't an argument against keeping the features, only 
about a mask param.
So how about passing a struct which has bools instead of a mask?
Something like this:

struct status_info_req_t
{
bool get_modifications;
bool get_needslock;
bool get_filesize;
bool get_whatever;
}

With this, you immediately can see in the code what each flag is used 
for and what it means.

> At this point, use libsvn_wc APIs if you need to get something done. I
> don't care if you need to use them. Others say "only client".
> Whatever. The simple fact is: the API isn't "done", so use whatever
> you need.

And I thought this mailing list was to be used to discuss stuff, raise 
concerns and get to an agreement on how things should be done. Silly me. 
Seems it's only "take what's there and stop complaining".


Your attitude sucks.


Stefan

P.S. Since I have now a week off from work, I'll take the time to think 
about whether I want to keep working on TSVN. From what I see on trunk, 
what I get here about where 1.7 is going and the attitude it doesn't 
seem worth my effort anymore. Seems TSVN 1.7 can only get worse than 
1.6, so what's the point.
Don't expect anymore posts from me, at least until I made my decision.

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Jun 18, 2010 at 12:28, Stefan Küng <to...@gmail.com> wrote:
>...
> Since you obviously haven't read my post here:
> http://svn.haxx.se/dev/archive-2010-06/0250.shtml

I read it. I read practically every post on dev@ *AND* most of the
posts to commits@.

I didn't respond because I didn't like the idea. We've used flag-based
APIs in the past (entry_modify comes to mind), and they all tend to
suck in the long run. They tend to hide what the function is used for,
and make it very hard to reason about the function. For example,
searching thru the code, I find the following:

  svn_wc__entry_modify(db, local_abspath, svn_kind_unknown,
&tmp_entry, modify_flags, scratch_pool);

That tells me nothing about what that call is doing. So I need to open
an editor, look for the call, and track down all assignments to
modify_flags. And I also need to ensure each member of tmp_entry
denoted by modify_flags has been assigned.

Just way too hard to work with, and so we're undoing all of those in libsvn_wc.

At this point, use libsvn_wc APIs if you need to get something done. I
don't care if you need to use them. Others say "only client".
Whatever. The simple fact is: the API isn't "done", so use whatever
you need.

Cheers,
-g

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 18.06.2010 06:46, Greg Stein wrote:
[snip]
> on. Offer some suggestions for improvements, rather than "wah wah, I
> don't like it".

Thanks for the insult. Hope you feel better now.

Since you obviously haven't read my post here:
http://svn.haxx.se/dev/archive-2010-06/0250.shtml

So again, I suggest the following:

Keep the features svn_client_status() currently has with all the 
information but still allow it to be much faster by introducing a mask 
parameter which the caller uses to specify which info the status API has 
to fetch. For an example, see the links I posted in the mentioned post 
above.
That way, you still can have the status API be very fast and only fetch 
the 'fast' info that's needed, but still get the 'slow' if the caller 
requests it.


So what's wrong with this suggestion? But please, if you have some 
concerns, say so with something to back it up. Stating it's against some 
policy I've never heard of and can't find in any of the svn docs is 
*not* a reason.


Stefan


-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Jun 17, 2010 at 16:48, Stefan Küng <to...@gmail.com> wrote:
> On 17.06.2010 22:35, Bert Huijben wrote:
>>> Sure, I said before I can get any information myself, but only with
>>> a severe performance hit. Because every time I call an svn_client_*
>>> API, it walks the whole tree again, opens the files again, opens
>>> the db again and again, ...
>>
>> svn_client doesn't open the db again. (The handle is cached in the
>> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
>> on your call it doesn't traverse any other nodes then the one you
>> specify.
>
> But I need that information for all entries in the tree. So I have to
> traverse the whole tree again.
> Tree walks are expensive.

That is totally ridiculous. No. You do NOT have to walk the tree again.

The idea is: the status callback is invoked with the *limited* status
information. If you want more, then from *within* that callback, make
a function call back into the API to get the information you need.

Look at that! No additional tree walks!

> I don't have a problem calling another API to get additional info where i
> need it only for one or just a few items. But most of the time, I need that
> information for the whole tree.

Great. And if you were *listening*, Bert has said we'll do that at the
client level. And even if we don't, then *you* can have a simple
callback handler that grabs the extra information.

This is not rocket science.

And as Mike pointed out: we'd not going to screw you over. So stop
your whining, make the extra calls in your callback, and let's move
on. Offer some suggestions for improvements, rather than "wah wah, I
don't like it".

Simple fact is: we're going for *fast*. If you want something less
fast, then you can do that on your own side of the callbacks. All the
information is there, and a call into the APIs is no more expensive
than what our old code used to do. When it gets down to it, the SAME
function is used to do the "is modified?" check. So what the hell are
you complaining about, if you have to make the call instead of the svn
library? Not to mention, Bert saying that libsvn_client will do all
that for you, anyways, in a consistent fashion.

Stop the noise. Move along.

-g

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 22:35, Bert Huijben wrote:

>> Sure, I said before I can get any information myself, but only with
>> a severe performance hit. Because every time I call an svn_client_*
>> API, it walks the whole tree again, opens the files again, opens
>> the db again and again, ...
>
> svn_client doesn't open the db again. (The handle is cached in the
> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
> on your call it doesn't traverse any other nodes then the one you
> specify.

But I need that information for all entries in the tree. So I have to 
traverse the whole tree again.
Tree walks are expensive.

I don't have a problem calling another API to get additional info where 
i need it only for one or just a few items. But most of the time, I need 
that information for the whole tree.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 22:35, Bert Huijben wrote:

>> Sure, I said before I can get any information myself, but only with
>> a severe performance hit. Because every time I call an svn_client_*
>> API, it walks the whole tree again, opens the files again, opens
>> the db again and again, ...
>
> svn_client doesn't open the db again. (The handle is cached in the
> svn_client_ctx_t's wc_ctx field) and if you set a proper depth value
> on your call it doesn't traverse any other nodes then the one you
> specify.

But I need that information for all entries in the tree. So I have to 
traverse the whole tree again.
Tree walks are expensive.

I don't have a problem calling another API to get additional info where 
i need it only for one or just a few items. But most of the time, I need 
that information for the whole tree.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: donderdag 17 juni 2010 22:28
> To: Bert Huijben
> Cc: 'C. Michael Pilato'; 'Subversion Development'
> Subject: Re: status information
> 
> On 17.06.2010 22:08, Bert Huijben wrote:
> 
> >> May I suggest an approach that doesn't force me to find expensive
> >> workarounds for more and more missing information the svn APIs
> >> return: Use a mask parameter which specifies which members of a
> >> returned struct get filled in by the API and which ones are not.
> >> That way, the less one specifies in the mask the faster the API
> >> returns, but if necessary everything necessary can still be
> >> returned in one tree walk without the need to invoke several other
> >> APIs which also do the very same tree walk.
> >>
> >> If you need an example, have a look at TVM_GETITEM and the
> >> TVITEMEX struct:
> >> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
> >> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
> >> The mask member specifies which members of the struct will be
> >> filled by the TVM_GETITEM call.
> >>
> >> This also has the advantage that new data can be added later
> >> without changing the API.
> >
> > Using flags for this is against the subversion principles, so this
> > would require adding booleans to svn_client_statusX() for every flag
> > on what you want and don't want.
> 
> Where's this policy in writing?
> Using booleans is good as long as there are only few. If more are
> needed, then a mask is the much better option.
> And I think policies have to change if sticking to them would mean
> serious drawbacks.
> 
> > For WC-NG we used a similar approach, but it only works when you are
> > calling an API: allow output arguments to be NULL. But this requires
> > that you are calling the api from the callback. (Which was exactly
> > the plan for every expensive operation; especially if you can't say
> > which value you want for which file/directory)
> 
> But calling an API from a callback either means using API's from
> sub-libraries (like libsvn_wc), or getting a huge performance hit by
> using svn_client_ API's.
> 
> > But before continuing this conversation, can you check if
> > svn_client_status_t (in svn_client.h) contains what you need?
> 
> As I said before, I'd like the svn:needs-lock and the size param back.
> 
> > I think we can safely remove the locked boolean from this struct
> > (which requires an additional DB lookup per directory) and you asked
> > for the translated filesize.
> >
> > (Not sure if the filesize that is still a useful addition for every
> > result as you can obtain such values very cheap via different APIs
> > now, because we don't have to read all entries at once any more. And
> > I don't think you use that value while walking the status, but only
> > after it returned)
> 
> Sure, I said before I can get any information myself, but only with a
> severe performance hit. Because every time I call an svn_client_* API,
> it walks the whole tree again, opens the files again, opens the db
> again
> and again, ...

svn_client doesn't open the db again. (The handle is cached in the svn_client_ctx_t's wc_ctx field) and if you set a proper depth value on your call it doesn't traverse any other nodes then the one you specify.

Subversion 1.6 did always read all the entries of a directory in memory, but with the sqlite databases this is no longer necessary. (Unless some code explicitly uses the old entries apis). As far as I can tell libsvn_client and libsvn_wc don't do that from any of the not-deprecated code paths.

(A few weeks ago the code to detect a working copy root still read the names of all nodes in the parent directory in memory, but this slowdown on extreme large directories has been removed)

	Bert

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Küng wrote:
> So for me, 1.7 doesn't look like an improvement at all, no matter how
> great WC-NG will get. There aren't any new features in it (yet) which I
> haven't already used in 1.6, but I have to drop more and more existing
> features.
> 
> And from your comments I get the impression that it will get worse (for
> me and TSVN) as time goes by.

Let's put an end to this right now, shall we?

TortoiseSVN has been and continues to be a significant source of the
overwhelming success that Subversion enjoys today, and I don't see that
changing unless that change is forced.  It would be incredibly foolish for
this community to make API changes that serve only to lessen TortoiseSVN's
ability to remain a viable Subversion client on a critical platform.

There is a solution to be found here.  We will find it, and we will find it
regardless of silly nuances and arbitrarily nods toward "the way we've
always done it".  If that way isn't the way of sane progress, it's the wrong
route for this project.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: donderdag 17 juni 2010 22:28
> To: Bert Huijben
> Cc: 'C. Michael Pilato'; 'Subversion Development'
> Subject: Re: status information
> 
> On 17.06.2010 22:08, Bert Huijben wrote:
> 
> >> May I suggest an approach that doesn't force me to find expensive
> >> workarounds for more and more missing information the svn APIs
> >> return: Use a mask parameter which specifies which members of a
> >> returned struct get filled in by the API and which ones are not.
> >> That way, the less one specifies in the mask the faster the API
> >> returns, but if necessary everything necessary can still be
> >> returned in one tree walk without the need to invoke several other
> >> APIs which also do the very same tree walk.
> >>
> >> If you need an example, have a look at TVM_GETITEM and the
> >> TVITEMEX struct:
> >> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
> >> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
> >> The mask member specifies which members of the struct will be
> >> filled by the TVM_GETITEM call.
> >>
> >> This also has the advantage that new data can be added later
> >> without changing the API.
> >
> > Using flags for this is against the subversion principles, so this
> > would require adding booleans to svn_client_statusX() for every flag
> > on what you want and don't want.
> 
> Where's this policy in writing?
> Using booleans is good as long as there are only few. If more are
> needed, then a mask is the much better option.
> And I think policies have to change if sticking to them would mean
> serious drawbacks.
> 
> > For WC-NG we used a similar approach, but it only works when you are
> > calling an API: allow output arguments to be NULL. But this requires
> > that you are calling the api from the callback. (Which was exactly
> > the plan for every expensive operation; especially if you can't say
> > which value you want for which file/directory)
> 
> But calling an API from a callback either means using API's from
> sub-libraries (like libsvn_wc), or getting a huge performance hit by
> using svn_client_ API's.
> 
> > But before continuing this conversation, can you check if
> > svn_client_status_t (in svn_client.h) contains what you need?
> 
> As I said before, I'd like the svn:needs-lock and the size param back.
> 
> > I think we can safely remove the locked boolean from this struct
> > (which requires an additional DB lookup per directory) and you asked
> > for the translated filesize.
> >
> > (Not sure if the filesize that is still a useful addition for every
> > result as you can obtain such values very cheap via different APIs
> > now, because we don't have to read all entries at once any more. And
> > I don't think you use that value while walking the status, but only
> > after it returned)
> 
> Sure, I said before I can get any information myself, but only with a
> severe performance hit. Because every time I call an svn_client_* API,
> it walks the whole tree again, opens the files again, opens the db
> again
> and again, ...

svn_client doesn't open the db again. (The handle is cached in the svn_client_ctx_t's wc_ctx field) and if you set a proper depth value on your call it doesn't traverse any other nodes then the one you specify.

Subversion 1.6 did always read all the entries of a directory in memory, but with the sqlite databases this is no longer necessary. (Unless some code explicitly uses the old entries apis). As far as I can tell libsvn_client and libsvn_wc don't do that from any of the not-deprecated code paths.

(A few weeks ago the code to detect a working copy root still read the names of all nodes in the parent directory in memory, but this slowdown on extreme large directories has been removed)

	Bert


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 22:08, Bert Huijben wrote:

>> May I suggest an approach that doesn't force me to find expensive
>> workarounds for more and more missing information the svn APIs
>> return: Use a mask parameter which specifies which members of a
>> returned struct get filled in by the API and which ones are not.
>> That way, the less one specifies in the mask the faster the API
>> returns, but if necessary everything necessary can still be
>> returned in one tree walk without the need to invoke several other
>> APIs which also do the very same tree walk.
>>
>> If you need an example, have a look at TVM_GETITEM and the
>> TVITEMEX struct:
>> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
>> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
>> The mask member specifies which members of the struct will be
>> filled by the TVM_GETITEM call.
>>
>> This also has the advantage that new data can be added later
>> without changing the API.
>
> Using flags for this is against the subversion principles, so this
> would require adding booleans to svn_client_statusX() for every flag
> on what you want and don't want.

Where's this policy in writing?
Using booleans is good as long as there are only few. If more are 
needed, then a mask is the much better option.
And I think policies have to change if sticking to them would mean 
serious drawbacks.

> For WC-NG we used a similar approach, but it only works when you are
> calling an API: allow output arguments to be NULL. But this requires
> that you are calling the api from the callback. (Which was exactly
> the plan for every expensive operation; especially if you can't say
> which value you want for which file/directory)

But calling an API from a callback either means using API's from 
sub-libraries (like libsvn_wc), or getting a huge performance hit by 
using svn_client_ API's.

> But before continuing this conversation, can you check if
> svn_client_status_t (in svn_client.h) contains what you need?

As I said before, I'd like the svn:needs-lock and the size param back.

> I think we can safely remove the locked boolean from this struct
> (which requires an additional DB lookup per directory) and you asked
> for the translated filesize.
>
> (Not sure if the filesize that is still a useful addition for every
> result as you can obtain such values very cheap via different APIs
> now, because we don't have to read all entries at once any more. And
> I don't think you use that value while walking the status, but only
> after it returned)

Sure, I said before I can get any information myself, but only with a 
severe performance hit. Because every time I call an svn_client_* API, 
it walks the whole tree again, opens the files again, opens the db again 
and again, ...

I'm currently facing big problems because of this. The svn lib is much 
slower on trunk as it is (yes, it should improve soon, but we'll have to 
first see how much it will improve). And you guys removing more and more 
information I need and rely on from APIs means I have to either add more 
and more expensive API calls or drop whole features.
For the missing svn:needs-lock, I did some testing and the performance 
hit is so severe that I have to drop several features in TSVN because of 
this.
So for me, 1.7 doesn't look like an improvement at all, no matter how 
great WC-NG will get. There aren't any new features in it (yet) which I 
haven't already used in 1.6, but I have to drop more and more existing 
features.

And from your comments I get the impression that it will get worse (for 
me and TSVN) as time goes by.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 22:08, Bert Huijben wrote:

>> May I suggest an approach that doesn't force me to find expensive
>> workarounds for more and more missing information the svn APIs
>> return: Use a mask parameter which specifies which members of a
>> returned struct get filled in by the API and which ones are not.
>> That way, the less one specifies in the mask the faster the API
>> returns, but if necessary everything necessary can still be
>> returned in one tree walk without the need to invoke several other
>> APIs which also do the very same tree walk.
>>
>> If you need an example, have a look at TVM_GETITEM and the
>> TVITEMEX struct:
>> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
>> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
>> The mask member specifies which members of the struct will be
>> filled by the TVM_GETITEM call.
>>
>> This also has the advantage that new data can be added later
>> without changing the API.
>
> Using flags for this is against the subversion principles, so this
> would require adding booleans to svn_client_statusX() for every flag
> on what you want and don't want.

Where's this policy in writing?
Using booleans is good as long as there are only few. If more are 
needed, then a mask is the much better option.
And I think policies have to change if sticking to them would mean 
serious drawbacks.

> For WC-NG we used a similar approach, but it only works when you are
> calling an API: allow output arguments to be NULL. But this requires
> that you are calling the api from the callback. (Which was exactly
> the plan for every expensive operation; especially if you can't say
> which value you want for which file/directory)

But calling an API from a callback either means using API's from 
sub-libraries (like libsvn_wc), or getting a huge performance hit by 
using svn_client_ API's.

> But before continuing this conversation, can you check if
> svn_client_status_t (in svn_client.h) contains what you need?

As I said before, I'd like the svn:needs-lock and the size param back.

> I think we can safely remove the locked boolean from this struct
> (which requires an additional DB lookup per directory) and you asked
> for the translated filesize.
>
> (Not sure if the filesize that is still a useful addition for every
> result as you can obtain such values very cheap via different APIs
> now, because we don't have to read all entries at once any more. And
> I don't think you use that value while walking the status, but only
> after it returned)

Sure, I said before I can get any information myself, but only with a 
severe performance hit. Because every time I call an svn_client_* API, 
it walks the whole tree again, opens the files again, opens the db again 
and again, ...

I'm currently facing big problems because of this. The svn lib is much 
slower on trunk as it is (yes, it should improve soon, but we'll have to 
first see how much it will improve). And you guys removing more and more 
information I need and rely on from APIs means I have to either add more 
and more expensive API calls or drop whole features.
For the missing svn:needs-lock, I did some testing and the performance 
hit is so severe that I have to drop several features in TSVN because of 
this.
So for me, 1.7 doesn't look like an improvement at all, no matter how 
great WC-NG will get. There aren't any new features in it (yet) which I 
haven't already used in 1.6, but I have to drop more and more existing 
features.

And from your comments I get the impression that it will get worse (for 
me and TSVN) as time goes by.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: donderdag 17 juni 2010 20:55
> To: C. Michael Pilato
> Cc: Bert Huijben; 'Subversion Development'
> Subject: Re: status information
> 
> On 17.06.2010 17:52, C. Michael Pilato wrote:
> > Bert Huijben wrote:
> >> Most simple clients can just use svn_client_status5() and trust the
> results
> >> to be complete like they were used to, but more advanced clients can
> use
> >> svn_wc_walk_status() to get higher performance.
> >>
> >> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can
> just show
> >> the conflicted glyph when it sees the conflict on a 2 GB file,
> instead of
> >> also comparing that file against its base version. And if you run
> status
> >> just to show a glyph for a modification on a subdirectory it is not
> >> necessary to perform comparation on the rest of the files, when you
> found a
> >> single change. (But you would want the conflict results)
> >
> > I really, really don't like the idea of encouraging the continued use
> of WC
> > APIs by third-party consumers.  We should begin *right now*
> discouraging its
> > use and simply improving the svn_client API to better serve our
> various
> > clients.  (I thought we had, in general, consensed around this plan
> already,
> > but I guess I was wrong.)
> 
> Yes, the discussion is here:
> http://svn.haxx.se/dev/archive-2010-04/0171.shtml
> (the apache mailing list archive still doesn't work for me).
> 
> May I suggest an approach that doesn't force me to find expensive
> workarounds for more and more missing information the svn APIs return:
> Use a mask parameter which specifies which members of a returned struct
> get filled in by the API and which ones are not. That way, the less one
> specifies in the mask the faster the API returns, but if necessary
> everything necessary can still be returned in one tree walk without the
> need to invoke several other APIs which also do the very same tree
> walk.
> 
> If you need an example, have a look at TVM_GETITEM and the TVITEMEX
> struct:
> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
> The mask member specifies which members of the struct will be filled by
> the TVM_GETITEM call.
> 
> This also has the advantage that new data can be added later without
> changing the API.

Using flags for this is against the subversion principles, so this would require adding booleans to svn_client_statusX() for every flag on what you want and don't want. 

For WC-NG we used a similar approach, but it only works when you are calling an API: allow output arguments to be NULL. But this requires that you are calling the api from the callback. (Which was exactly the plan for every expensive operation; especially if you can't say which value you want for which file/directory)


But before continuing this conversation, can you check if svn_client_status_t (in svn_client.h) contains what you need?

I think we can safely remove the locked boolean from this struct (which requires an additional DB lookup per directory) and you asked for the translated filesize.

(Not sure if the filesize that is still a useful addition for every result as you can obtain such values very cheap via different APIs now, because we don't have to read all entries at once any more. And I don't think you use that value while walking the status, but only after it returned)

	Bert


RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: donderdag 17 juni 2010 20:55
> To: C. Michael Pilato
> Cc: Bert Huijben; 'Subversion Development'
> Subject: Re: status information
> 
> On 17.06.2010 17:52, C. Michael Pilato wrote:
> > Bert Huijben wrote:
> >> Most simple clients can just use svn_client_status5() and trust the
> results
> >> to be complete like they were used to, but more advanced clients can
> use
> >> svn_wc_walk_status() to get higher performance.
> >>
> >> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can
> just show
> >> the conflicted glyph when it sees the conflict on a 2 GB file,
> instead of
> >> also comparing that file against its base version. And if you run
> status
> >> just to show a glyph for a modification on a subdirectory it is not
> >> necessary to perform comparation on the rest of the files, when you
> found a
> >> single change. (But you would want the conflict results)
> >
> > I really, really don't like the idea of encouraging the continued use
> of WC
> > APIs by third-party consumers.  We should begin *right now*
> discouraging its
> > use and simply improving the svn_client API to better serve our
> various
> > clients.  (I thought we had, in general, consensed around this plan
> already,
> > but I guess I was wrong.)
> 
> Yes, the discussion is here:
> http://svn.haxx.se/dev/archive-2010-04/0171.shtml
> (the apache mailing list archive still doesn't work for me).
> 
> May I suggest an approach that doesn't force me to find expensive
> workarounds for more and more missing information the svn APIs return:
> Use a mask parameter which specifies which members of a returned struct
> get filled in by the API and which ones are not. That way, the less one
> specifies in the mask the faster the API returns, but if necessary
> everything necessary can still be returned in one tree walk without the
> need to invoke several other APIs which also do the very same tree
> walk.
> 
> If you need an example, have a look at TVM_GETITEM and the TVITEMEX
> struct:
> http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
> http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
> The mask member specifies which members of the struct will be filled by
> the TVM_GETITEM call.
> 
> This also has the advantage that new data can be added later without
> changing the API.

Using flags for this is against the subversion principles, so this would require adding booleans to svn_client_statusX() for every flag on what you want and don't want. 

For WC-NG we used a similar approach, but it only works when you are calling an API: allow output arguments to be NULL. But this requires that you are calling the api from the callback. (Which was exactly the plan for every expensive operation; especially if you can't say which value you want for which file/directory)


But before continuing this conversation, can you check if svn_client_status_t (in svn_client.h) contains what you need?

I think we can safely remove the locked boolean from this struct (which requires an additional DB lookup per directory) and you asked for the translated filesize.

(Not sure if the filesize that is still a useful addition for every result as you can obtain such values very cheap via different APIs now, because we don't have to read all entries at once any more. And I don't think you use that value while walking the status, but only after it returned)

	Bert

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 17:52, C. Michael Pilato wrote:
> Bert Huijben wrote:
>> Most simple clients can just use svn_client_status5() and trust the results
>> to be complete like they were used to, but more advanced clients can use
>> svn_wc_walk_status() to get higher performance.
>>
>> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
>> the conflicted glyph when it sees the conflict on a 2 GB file, instead of
>> also comparing that file against its base version. And if you run status
>> just to show a glyph for a modification on a subdirectory it is not
>> necessary to perform comparation on the rest of the files, when you found a
>> single change. (But you would want the conflict results)
>
> I really, really don't like the idea of encouraging the continued use of WC
> APIs by third-party consumers.  We should begin *right now* discouraging its
> use and simply improving the svn_client API to better serve our various
> clients.  (I thought we had, in general, consensed around this plan already,
> but I guess I was wrong.)

Yes, the discussion is here:
http://svn.haxx.se/dev/archive-2010-04/0171.shtml
(the apache mailing list archive still doesn't work for me).

May I suggest an approach that doesn't force me to find expensive 
workarounds for more and more missing information the svn APIs return:
Use a mask parameter which specifies which members of a returned struct 
get filled in by the API and which ones are not. That way, the less one 
specifies in the mask the faster the API returns, but if necessary 
everything necessary can still be returned in one tree walk without the 
need to invoke several other APIs which also do the very same tree walk.

If you need an example, have a look at TVM_GETITEM and the TVITEMEX struct:
http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
The mask member specifies which members of the struct will be filled by 
the TVM_GETITEM call.

This also has the advantage that new data can be added later without 
changing the API.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 17.06.2010 17:52, C. Michael Pilato wrote:
> Bert Huijben wrote:
>> Most simple clients can just use svn_client_status5() and trust the results
>> to be complete like they were used to, but more advanced clients can use
>> svn_wc_walk_status() to get higher performance.
>>
>> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
>> the conflicted glyph when it sees the conflict on a 2 GB file, instead of
>> also comparing that file against its base version. And if you run status
>> just to show a glyph for a modification on a subdirectory it is not
>> necessary to perform comparation on the rest of the files, when you found a
>> single change. (But you would want the conflict results)
>
> I really, really don't like the idea of encouraging the continued use of WC
> APIs by third-party consumers.  We should begin *right now* discouraging its
> use and simply improving the svn_client API to better serve our various
> clients.  (I thought we had, in general, consensed around this plan already,
> but I guess I was wrong.)

Yes, the discussion is here:
http://svn.haxx.se/dev/archive-2010-04/0171.shtml
(the apache mailing list archive still doesn't work for me).

May I suggest an approach that doesn't force me to find expensive 
workarounds for more and more missing information the svn APIs return:
Use a mask parameter which specifies which members of a returned struct 
get filled in by the API and which ones are not. That way, the less one 
specifies in the mask the faster the API returns, but if necessary 
everything necessary can still be returned in one tree walk without the 
need to invoke several other APIs which also do the very same tree walk.

If you need an example, have a look at TVM_GETITEM and the TVITEMEX struct:
http://msdn.microsoft.com/en-us/library/bb773758%28v=VS.85%29.aspx
http://msdn.microsoft.com/en-us/library/bb773459%28v=VS.85%29.aspx
The mask member specifies which members of the struct will be filled by 
the TVM_GETITEM call.

This also has the advantage that new data can be added later without 
changing the API.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
> Most simple clients can just use svn_client_status5() and trust the results
> to be complete like they were used to, but more advanced clients can use
> svn_wc_walk_status() to get higher performance. 
> 
> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
> the conflicted glyph when it sees the conflict on a 2 GB file, instead of
> also comparing that file against its base version. And if you run status
> just to show a glyph for a modification on a subdirectory it is not
> necessary to perform comparation on the rest of the files, when you found a
> single change. (But you would want the conflict results)

I really, really don't like the idea of encouraging the continued use of WC
APIs by third-party consumers.  We should begin *right now* discouraging its
use and simply improving the svn_client API to better serve our various
clients.  (I thought we had, in general, consensed around this plan already,
but I guess I was wrong.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On Thu, Jun 17, 2010 at 11:49, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
>> Sent: woensdag 16 juni 2010 22:14
>> To: Bert Huijben; Subversion Development
>> Subject: Re: status information
>>
>> On 16.06.2010 21:40, Bert Huijben wrote:
>>
>> > The plan is to remove even more expensive members from
>> svn_wc_status3_t, to
>> > make it just return cheap information. (I think working_size is
>> available,
>> > so we can add that specific one; but the scan if the file has been
>> modified
>> > on disk will be removed).
>>
>> Huh? The scan whether a file was modified gets removed? Then what is
>> the
>> status call for if not to get the text and property status?
>> If you really want an API which returns less info faster, please create
>> a new API for it (or a flag in the svn_client_status() call to omit the
>> more expensive checks). But seriously: whether a file is modified *is*
>> the status of a file. If you remove that info, it's not a status API
>> anymore but a 'svn_client_fastWcInfo()' or something like this.
>>
>> I think having svn_client_status() not return that information would
>> lead to much confusion about the term 'status'.
>
> The problem here is that svn_client_statusX() still uses svn_wc_status3_t.
> The idea is to make this function use a new svn_client_status_t structure
> which is different from svn_wc_status3_t in that it contains more 'expensive
> data'.

Ok, no problem with that.
What I have a problem with is that you said you wanted to remove the
modified info. But the modified info *is* the status. And the 'status'
API must return that status. Otherwise it's not a status API but
something else.
So please, no matter what you name that new structure: it must provide
the status info. If not, use another API.

> svn_wc_*status() will just give you the cheap data and information on which
> expensive data you can retrieve yourself. The problem with the current
> svn_wc_status infrastructure is that it always does all the work and in some
> cases then even throws away the result. (E.g. when you find a conflict the
> text modifications are already calculated, but not used for the status
> result).
>
> Most simple clients can just use svn_client_status5() and trust the results
> to be complete like they were used to, but more advanced clients can use
> svn_wc_walk_status() to get higher performance.
>
> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
> the conflicted glyph when it sees the conflict on a 2 GB file, instead of
> also comparing that file against its base version. And if you run status
> just to show a glyph for a modification on a subdirectory it is not
> necessary to perform comparation on the rest of the files, when you found a
> single change. (But you would want the conflict results)

Sure, that's one way of dealing with this.
But it requires that every svn client has to implement the same code
itself, instead of having it in the svn lib.

Why not just add a mask param to the svn status API which specifies
which info the client requires and have the svn lib fetch all the
required info in *one single* wc crawl? It can't get faster than that!
And all svn clients can be assured that they all get the *same* info
back. If it's not in the svn lib, clients might implement things
differently. For example, if clients have to compare WC/BASE
themselves to find out whether a file is modified, that only leads to
big troubles.

And due to the status callback, I can 'stop' a status crawl anytime I
like even with 1.6.x clients. That's nothing new.

>> The svn trunk now is a *lot* slower accessing the wc info than 1.6.x,
>> and even if that gets better (hopefully soon), forcing me to use three
>> API calls instead of one means an even worse slowdown. Even if trunk
>> was
>> as fast as 1.6.x, with those API changes it will take three times the
>> file accesses and crawls to get the same info as before.
>
> Are we on a single database?

No.

> Future performance is only speculation until we get there. For data layout

And that's what I'm worried about most. Nobody knows what the
performance will be when everything's finished. What if the
performance isn't much better than it is now? Even if the performance
gets to a point where it's only half as fast as 1.6, I wouldn't use
it.

> we are still where we were last September, but Greg says he is going to
> switch to in-db properties 'really soon now'.

Yes, I've seen the mails.

> The on-disk format is not stable and I wouldn't recommend using trunk in the
> current state unless you know what you are doing. (E.g. the locking system
> is completely broken)

I have to use it. If you really plan to have a one/two month grace
period before the 1.7 release (whenever that might be), that's not
enough time for me to catch up with TSVN. Also if I don't start using
it now, I won't have any chance to get some things changed if
necessary.

>> For example, to get the same info as with a status call before, I now
>> have to first get the status, then fetch the svn:needs-lock property on
>> every file separately. You can guess that this is a performance hit I
>> simply can not accept. That means I lose an important feature in TSVN,
>> something I'm absolutely sure many users will get very angry about.
>
> Properties will be much faster when we switch to the in-db properties. The
> current code writes all property data to files and the database, but only
> reads from the files.

Even if reading props is a zero-operation: it still requires me to
walk the whole wc again to find all files/folders to read the
properties from.

>> Also keep in mind that locking is one of the features of SVN that the
>> other (distributed) version control systems don't have, so that's one
>> of
>> the important things users choose svn over those.
>> Loosing that information (the svn:needs-lock info) or getting a severe
>> performance hit fetching it another way is just very bad. SVN should
>> make the features it stands out from the others make work well, not
>> worse. Or it will lose one of the most important advantages it has.
>
> I can't say what the performance hit is on retrieving in-db properties, but
> I assume that it will be much faster than opening a  file just to read one
> property.

But still: it requires some work to be done twice. If I'm not
completely mistaken, to find the property status of an item, you
already have to read at least some of the properties already. So you
have the db connection open, even to the items properties and you're
already reading (at least some of) them.

Stefan

-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: status information

Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
> Most simple clients can just use svn_client_status5() and trust the results
> to be complete like they were used to, but more advanced clients can use
> svn_wc_walk_status() to get higher performance. 
> 
> E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
> the conflicted glyph when it sees the conflict on a 2 GB file, instead of
> also comparing that file against its base version. And if you run status
> just to show a glyph for a modification on a subdirectory it is not
> necessary to perform comparation on the rest of the files, when you found a
> single change. (But you would want the conflict results)

I really, really don't like the idea of encouraging the continued use of WC
APIs by third-party consumers.  We should begin *right now* discouraging its
use and simply improving the svn_client API to better serve our various
clients.  (I thought we had, in general, consensed around this plan already,
but I guess I was wrong.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 16 juni 2010 22:14
> To: Bert Huijben; Subversion Development
> Subject: Re: status information
> 
> On 16.06.2010 21:40, Bert Huijben wrote:
> 
> > The plan is to remove even more expensive members from
> svn_wc_status3_t, to
> > make it just return cheap information. (I think working_size is
> available,
> > so we can add that specific one; but the scan if the file has been
> modified
> > on disk will be removed).
> 
> Huh? The scan whether a file was modified gets removed? Then what is
> the
> status call for if not to get the text and property status?
> If you really want an API which returns less info faster, please create
> a new API for it (or a flag in the svn_client_status() call to omit the
> more expensive checks). But seriously: whether a file is modified *is*
> the status of a file. If you remove that info, it's not a status API
> anymore but a 'svn_client_fastWcInfo()' or something like this.
> 
> I think having svn_client_status() not return that information would
> lead to much confusion about the term 'status'.

The problem here is that svn_client_statusX() still uses svn_wc_status3_t.
The idea is to make this function use a new svn_client_status_t structure
which is different from svn_wc_status3_t in that it contains more 'expensive
data'.

svn_wc_*status() will just give you the cheap data and information on which
expensive data you can retrieve yourself. The problem with the current
svn_wc_status infrastructure is that it always does all the work and in some
cases then even throws away the result. (E.g. when you find a conflict the
text modifications are already calculated, but not used for the status
result). 

Most simple clients can just use svn_client_status5() and trust the results
to be complete like they were used to, but more advanced clients can use
svn_wc_walk_status() to get higher performance. 

E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
the conflicted glyph when it sees the conflict on a 2 GB file, instead of
also comparing that file against its base version. And if you run status
just to show a glyph for a modification on a subdirectory it is not
necessary to perform comparation on the rest of the files, when you found a
single change. (But you would want the conflict results)


> > The plan is to introduce a new svn_client_status_t structure (for
> > svn_client_status) which will have more data then the
> svn_wc_status3_t
> > structure, but we haven't started on that one.

Update: I started on this now, just to document the plans.

> >
> > The assumption here is that users of the libsvn_wc api want direct
> access to
> > the working copy without any performance penalties they might not
> want.
> > (E.g. not scanning the file for changes, or parsing all the conflict
> > details)
> 
> I get that. But I suggest adding a separate API for this and not
> cripple
> the svn_client_status() for this. Status info means getting the info
> about which files are modified. That's what every svn user expects when
> talking about the status. So the API has to honor that.
> If you want a way to get less info faster, please create an API with
> another name for this.

As written above: svn_client_status5() will work just like
svn_client_statusX() in that it does all the checks. It will just pass a
svn_client_status_t instead of a svn_wc_status3_t. 

> > On the other side: users of the libsvn_client api wants to have
> access to
> > the direct usable combination of data, which might give less
> performance.
> 
> Yes, something I need to in some situations. But I'd like to have this
> information returned in one call if possible. Sure, I can now get the
> info I need in two or three svn API calls. But that means that to get
> the info, the wc is crawled three times instead of just once. And every
> time files are touched which is very expensive on Windows.
> 
> The svn trunk now is a *lot* slower accessing the wc info than 1.6.x,
> and even if that gets better (hopefully soon), forcing me to use three
> API calls instead of one means an even worse slowdown. Even if trunk
> was
> as fast as 1.6.x, with those API changes it will take three times the
> file accesses and crawls to get the same info as before.

Are we on a single database?

Future performance is only speculation until we get there. For data layout
we are still where we were last September, but Greg says he is going to
switch to in-db properties 'really soon now'.

The on-disk format is not stable and I wouldn't recommend using trunk in the
current state unless you know what you are doing. (E.g. the locking system
is completely broken)

> For example, to get the same info as with a status call before, I now
> have to first get the status, then fetch the svn:needs-lock property on
> every file separately. You can guess that this is a performance hit I
> simply can not accept. That means I lose an important feature in TSVN,
> something I'm absolutely sure many users will get very angry about.

Properties will be much faster when we switch to the in-db properties. The
current code writes all property data to files and the database, but only
reads from the files.

> Also keep in mind that locking is one of the features of SVN that the
> other (distributed) version control systems don't have, so that's one
> of
> the important things users choose svn over those.
> Loosing that information (the svn:needs-lock info) or getting a severe
> performance hit fetching it another way is just very bad. SVN should
> make the features it stands out from the others make work well, not
> worse. Or it will lose one of the most important advantages it has.

I can't say what the performance hit is on retrieving in-db properties, but
I assume that it will be much faster than opening a  file just to read one
property.

	Bert

> What I'm trying to say here: please don't cripple existing APIs but
> create new ones with new names, and have APIs that return the same info
> as before without a performance hit.
> 
> Stefan
> 
> --
>         ___
>    oo  // \\      "De Chelonian Mobile"
>   (_,\/ \_/ \     TortoiseSVN
>     \ \_/_\_/>    The coolest Interface to (Sub)Version Control
>     /_/   \_\     http://tortoisesvn.net

RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 16 juni 2010 22:14
> To: Bert Huijben; Subversion Development
> Subject: Re: status information
> 
> On 16.06.2010 21:40, Bert Huijben wrote:
> 
> > The plan is to remove even more expensive members from
> svn_wc_status3_t, to
> > make it just return cheap information. (I think working_size is
> available,
> > so we can add that specific one; but the scan if the file has been
> modified
> > on disk will be removed).
> 
> Huh? The scan whether a file was modified gets removed? Then what is
> the
> status call for if not to get the text and property status?
> If you really want an API which returns less info faster, please create
> a new API for it (or a flag in the svn_client_status() call to omit the
> more expensive checks). But seriously: whether a file is modified *is*
> the status of a file. If you remove that info, it's not a status API
> anymore but a 'svn_client_fastWcInfo()' or something like this.
> 
> I think having svn_client_status() not return that information would
> lead to much confusion about the term 'status'.

The problem here is that svn_client_statusX() still uses svn_wc_status3_t.
The idea is to make this function use a new svn_client_status_t structure
which is different from svn_wc_status3_t in that it contains more 'expensive
data'.

svn_wc_*status() will just give you the cheap data and information on which
expensive data you can retrieve yourself. The problem with the current
svn_wc_status infrastructure is that it always does all the work and in some
cases then even throws away the result. (E.g. when you find a conflict the
text modifications are already calculated, but not used for the status
result). 

Most simple clients can just use svn_client_status5() and trust the results
to be complete like they were used to, but more advanced clients can use
svn_wc_walk_status() to get higher performance. 

E.g. If TortoiseSVN or AnkhSVN switched to the wc apis, they can just show
the conflicted glyph when it sees the conflict on a 2 GB file, instead of
also comparing that file against its base version. And if you run status
just to show a glyph for a modification on a subdirectory it is not
necessary to perform comparation on the rest of the files, when you found a
single change. (But you would want the conflict results)


> > The plan is to introduce a new svn_client_status_t structure (for
> > svn_client_status) which will have more data then the
> svn_wc_status3_t
> > structure, but we haven't started on that one.

Update: I started on this now, just to document the plans.

> >
> > The assumption here is that users of the libsvn_wc api want direct
> access to
> > the working copy without any performance penalties they might not
> want.
> > (E.g. not scanning the file for changes, or parsing all the conflict
> > details)
> 
> I get that. But I suggest adding a separate API for this and not
> cripple
> the svn_client_status() for this. Status info means getting the info
> about which files are modified. That's what every svn user expects when
> talking about the status. So the API has to honor that.
> If you want a way to get less info faster, please create an API with
> another name for this.

As written above: svn_client_status5() will work just like
svn_client_statusX() in that it does all the checks. It will just pass a
svn_client_status_t instead of a svn_wc_status3_t. 

> > On the other side: users of the libsvn_client api wants to have
> access to
> > the direct usable combination of data, which might give less
> performance.
> 
> Yes, something I need to in some situations. But I'd like to have this
> information returned in one call if possible. Sure, I can now get the
> info I need in two or three svn API calls. But that means that to get
> the info, the wc is crawled three times instead of just once. And every
> time files are touched which is very expensive on Windows.
> 
> The svn trunk now is a *lot* slower accessing the wc info than 1.6.x,
> and even if that gets better (hopefully soon), forcing me to use three
> API calls instead of one means an even worse slowdown. Even if trunk
> was
> as fast as 1.6.x, with those API changes it will take three times the
> file accesses and crawls to get the same info as before.

Are we on a single database?

Future performance is only speculation until we get there. For data layout
we are still where we were last September, but Greg says he is going to
switch to in-db properties 'really soon now'.

The on-disk format is not stable and I wouldn't recommend using trunk in the
current state unless you know what you are doing. (E.g. the locking system
is completely broken)

> For example, to get the same info as with a status call before, I now
> have to first get the status, then fetch the svn:needs-lock property on
> every file separately. You can guess that this is a performance hit I
> simply can not accept. That means I lose an important feature in TSVN,
> something I'm absolutely sure many users will get very angry about.

Properties will be much faster when we switch to the in-db properties. The
current code writes all property data to files and the database, but only
reads from the files.

> Also keep in mind that locking is one of the features of SVN that the
> other (distributed) version control systems don't have, so that's one
> of
> the important things users choose svn over those.
> Loosing that information (the svn:needs-lock info) or getting a severe
> performance hit fetching it another way is just very bad. SVN should
> make the features it stands out from the others make work well, not
> worse. Or it will lose one of the most important advantages it has.

I can't say what the performance hit is on retrieving in-db properties, but
I assume that it will be much faster than opening a  file just to read one
property.

	Bert

> What I'm trying to say here: please don't cripple existing APIs but
> create new ones with new names, and have APIs that return the same info
> as before without a performance hit.
> 
> Stefan
> 
> --
>         ___
>    oo  // \\      "De Chelonian Mobile"
>   (_,\/ \_/ \     TortoiseSVN
>     \ \_/_\_/>    The coolest Interface to (Sub)Version Control
>     /_/   \_\     http://tortoisesvn.net


Re: status information

Posted by Stefan Küng <to...@gmail.com>.
On 16.06.2010 21:40, Bert Huijben wrote:

> The plan is to remove even more expensive members from svn_wc_status3_t, to
> make it just return cheap information. (I think working_size is available,
> so we can add that specific one; but the scan if the file has been modified
> on disk will be removed).

Huh? The scan whether a file was modified gets removed? Then what is the 
status call for if not to get the text and property status?
If you really want an API which returns less info faster, please create 
a new API for it (or a flag in the svn_client_status() call to omit the 
more expensive checks). But seriously: whether a file is modified *is* 
the status of a file. If you remove that info, it's not a status API 
anymore but a 'svn_client_fastWcInfo()' or something like this.

I think having svn_client_status() not return that information would 
lead to much confusion about the term 'status'.

> The plan is to introduce a new svn_client_status_t structure (for
> svn_client_status) which will have more data then the svn_wc_status3_t
> structure, but we haven't started on that one.
>
> The assumption here is that users of the libsvn_wc api want direct access to
> the working copy without any performance penalties they might not want.
> (E.g. not scanning the file for changes, or parsing all the conflict
> details)

I get that. But I suggest adding a separate API for this and not cripple 
the svn_client_status() for this. Status info means getting the info 
about which files are modified. That's what every svn user expects when 
talking about the status. So the API has to honor that.
If you want a way to get less info faster, please create an API with 
another name for this.

> On the other side: users of the libsvn_client api wants to have access to
> the direct usable combination of data, which might give less performance.

Yes, something I need to in some situations. But I'd like to have this 
information returned in one call if possible. Sure, I can now get the 
info I need in two or three svn API calls. But that means that to get 
the info, the wc is crawled three times instead of just once. And every 
time files are touched which is very expensive on Windows.

The svn trunk now is a *lot* slower accessing the wc info than 1.6.x, 
and even if that gets better (hopefully soon), forcing me to use three 
API calls instead of one means an even worse slowdown. Even if trunk was 
as fast as 1.6.x, with those API changes it will take three times the 
file accesses and crawls to get the same info as before.

For example, to get the same info as with a status call before, I now 
have to first get the status, then fetch the svn:needs-lock property on 
every file separately. You can guess that this is a performance hit I 
simply can not accept. That means I lose an important feature in TSVN, 
something I'm absolutely sure many users will get very angry about.

Also keep in mind that locking is one of the features of SVN that the 
other (distributed) version control systems don't have, so that's one of 
the important things users choose svn over those.
Loosing that information (the svn:needs-lock info) or getting a severe 
performance hit fetching it another way is just very bad. SVN should 
make the features it stands out from the others make work well, not 
worse. Or it will lose one of the most important advantages it has.

What I'm trying to say here: please don't cripple existing APIs but 
create new ones with new names, and have APIs that return the same info 
as before without a performance hit.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 16 juni 2010 21:29
> To: Subversion Development
> Subject: status information
> 
> Hi,
> 
> Since the svn_wc_status3_t struct is now missing the entry member, a
> lot
> of information previously available from a status call is missing.
> I can work around a lot of that information using other svn API calls,
> but there's at least two pieces of info I'd really have back in a
> status
> call:
> 
> working size : the size of the files returned by 'svn st'. Since a
> status call has to stat the files anyway, that information shouldn't be
> expensive to get.
> 
> svn:needs-lock : I know this is a property and hasn't really anything
> to
> do with the status of items. But it's a very important piece of
> information. In TSVN, we use this to show a special overlay icon so
> users can immediately see that they have to get a lock before trying to
> edit the file. The 'readonly' flag on such files isn't of much help
> here
> since most apps don't warn users about it until they try to save their
> changes.
> If it's too expensive to get the info whether the property is set,
> having the info whether the readonly filesystem flag is set would also
> work, and that info could be fetched with the file stat call that I
> think has to be done anyway.
> 
> Please consider adding these info pieces to the svn_wc_status3_t
> struct.

The plan is to remove even more expensive members from svn_wc_status3_t, to
make it just return cheap information. (I think working_size is available,
so we can add that specific one; but the scan if the file has been modified
on disk will be removed).

The plan is to introduce a new svn_client_status_t structure (for
svn_client_status) which will have more data then the svn_wc_status3_t
structure, but we haven't started on that one. 

The assumption here is that users of the libsvn_wc api want direct access to
the working copy without any performance penalties they might not want.
(E.g. not scanning the file for changes, or parsing all the conflict
details)

On the other side: users of the libsvn_client api wants to have access to
the direct usable combination of data, which might give less performance.

	Bert


RE: status information

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 16 juni 2010 21:29
> To: Subversion Development
> Subject: status information
> 
> Hi,
> 
> Since the svn_wc_status3_t struct is now missing the entry member, a
> lot
> of information previously available from a status call is missing.
> I can work around a lot of that information using other svn API calls,
> but there's at least two pieces of info I'd really have back in a
> status
> call:
> 
> working size : the size of the files returned by 'svn st'. Since a
> status call has to stat the files anyway, that information shouldn't be
> expensive to get.
> 
> svn:needs-lock : I know this is a property and hasn't really anything
> to
> do with the status of items. But it's a very important piece of
> information. In TSVN, we use this to show a special overlay icon so
> users can immediately see that they have to get a lock before trying to
> edit the file. The 'readonly' flag on such files isn't of much help
> here
> since most apps don't warn users about it until they try to save their
> changes.
> If it's too expensive to get the info whether the property is set,
> having the info whether the readonly filesystem flag is set would also
> work, and that info could be fetched with the file stat call that I
> think has to be done anyway.
> 
> Please consider adding these info pieces to the svn_wc_status3_t
> struct.

The plan is to remove even more expensive members from svn_wc_status3_t, to
make it just return cheap information. (I think working_size is available,
so we can add that specific one; but the scan if the file has been modified
on disk will be removed).

The plan is to introduce a new svn_client_status_t structure (for
svn_client_status) which will have more data then the svn_wc_status3_t
structure, but we haven't started on that one. 

The assumption here is that users of the libsvn_wc api want direct access to
the working copy without any performance penalties they might not want.
(E.g. not scanning the file for changes, or parsing all the conflict
details)

On the other side: users of the libsvn_client api wants to have access to
the direct usable combination of data, which might give less performance.

	Bert