You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by SteveKing <st...@gmx.ch> on 2005/07/23 07:50:25 UTC
[PATCH] avoid memory reallocation
Hi,
This is really a *very* little thing:
The CL client uses an array to store all the auth providers. The initial
size of that array is set to 10, but on Windows, there's one auth
provider more so the array is always reallocated (or I'm counting wrong,
could happen too).
This very little patch takes care of this.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by Branko Čibej <br...@xbc.nu>.
Michael W Thelen wrote:
>SteveKing wrote:
>
>
>>This is really a *very* little thing:
>>
>>The CL client uses an array to store all the auth providers. The initial
>>size of that array is set to 10, but on Windows, there's one auth
>>provider more so the array is always reallocated (or I'm counting wrong,
>>could happen too).
>>This very little patch takes care of this.
>>
>>
>
>Pinging this patch... Brane objected, saying that it's not worth the
>bother to change the initial size whenever a new auth provider is added.
> Steve responded that new auth providers are not added often, and the
>initial size already seems to be intended to accommodate all auth
>providers, which it doesn't quite do on Windows.
>
>My (relatively ignorant) opinion is that if we're going to use a magic
>number in the code (10), it might as well be large enough to accommodate
>all auth providers on Windows (11), so the array doesn't need to be
>reallocated. There are 11 calls to APR_ARRAY_PUSH in the code... the
>array may as well be preallocated to 11, right? I don't see any
>downsides to applying this patch. Are there any?
>
>
There are no downsides.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 19 Aug 2005, Erik Huelsmann wrote:
> On 8/18/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> > I think this can be fixed the next time we add a provider.
>
> Given that there already is a patch, why postpone?
>
No reasons other than laziness and other priorities. But, soon this
discussion has taken more time than it would have taken to commit it at
the beginning...
> > But you're right that there is nothin gwrong with it.
>
> Ok. In that case, anybody object if I commit the patch tonight?
>
+0.
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by Erik Huelsmann <eh...@gmail.com>.
On 8/18/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> On Thu, 18 Aug 2005, Michael W Thelen wrote:
>
> > SteveKing wrote:
> > > This is really a *very* little thing:
> > >
> > > The CL client uses an array to store all the auth providers. The initial
> > > size of that array is set to 10, but on Windows, there's one auth
> > > provider more so the array is always reallocated (or I'm counting wrong,
> > > could happen too).
> > > This very little patch takes care of this.
> >
> > Pinging this patch... Brane objected, saying that it's not worth the
> > bother to change the initial size whenever a new auth provider is added.
> > Steve responded that new auth providers are not added often, and the
> > initial size already seems to be intended to accommodate all auth
> > providers, which it doesn't quite do on Windows.
> >
> I think this can be fixed the next time we add a provider.
Given that there already is a patch, why postpone?
> But you're right that there is nothin gwrong with it.
Ok. In that case, anybody object if I commit the patch tonight?
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Thu, 18 Aug 2005, Michael W Thelen wrote:
> SteveKing wrote:
> > This is really a *very* little thing:
> >
> > The CL client uses an array to store all the auth providers. The initial
> > size of that array is set to 10, but on Windows, there's one auth
> > provider more so the array is always reallocated (or I'm counting wrong,
> > could happen too).
> > This very little patch takes care of this.
>
> Pinging this patch... Brane objected, saying that it's not worth the
> bother to change the initial size whenever a new auth provider is added.
> Steve responded that new auth providers are not added often, and the
> initial size already seems to be intended to accommodate all auth
> providers, which it doesn't quite do on Windows.
>
I think this can be fixed the next time we add a provider.
But you're right that there is nothin gwrong with it.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by Michael W Thelen <mi...@pietdepsi.com>.
SteveKing wrote:
> This is really a *very* little thing:
>
> The CL client uses an array to store all the auth providers. The initial
> size of that array is set to 10, but on Windows, there's one auth
> provider more so the array is always reallocated (or I'm counting wrong,
> could happen too).
> This very little patch takes care of this.
Pinging this patch... Brane objected, saying that it's not worth the
bother to change the initial size whenever a new auth provider is added.
Steve responded that new auth providers are not added often, and the
initial size already seems to be intended to accommodate all auth
providers, which it doesn't quite do on Windows.
My (relatively ignorant) opinion is that if we're going to use a magic
number in the code (10), it might as well be large enough to accommodate
all auth providers on Windows (11), so the array doesn't need to be
reallocated. There are 11 calls to APR_ARRAY_PUSH in the code... the
array may as well be preallocated to 11, right? I don't see any
downsides to applying this patch. Are there any?
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by SteveKing <st...@gmx.ch>.
Branko Čibej wrote:
>> This is really a *very* little thing:
>>
>> The CL client uses an array to store all the auth providers. The
>> initial size of that array is set to 10, but on Windows, there's one
>> auth provider more so the array is always reallocated (or I'm counting
>> wrong, could happen too).
>> This very little patch takes care of this.
>
>
> Perhaps it does, but why bother? Do you propose we change the initial
> size of the array every time we add an auth provider. So the
> reallocation may be ugly, but it doesn't affect the behaviour of the
> client in any measurable way.
Well, I don't think that you'll be adding new auth providers every few
days, so adjusting the initial size of the array is really not too much
work. And sure, it doesn't affect the behaviour of the client at all,
but as you said it's ugly. After all, the initial size is set to 10 - if
you don't care about reallocation you could have chosen 1 for that
initial size.
Anyway, if you don't like it, don't use it.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] avoid memory reallocation
Posted by Branko Čibej <br...@xbc.nu>.
SteveKing wrote:
> Hi,
>
> This is really a *very* little thing:
>
> The CL client uses an array to store all the auth providers. The
> initial size of that array is set to 10, but on Windows, there's one
> auth provider more so the array is always reallocated (or I'm counting
> wrong, could happen too).
> This very little patch takes care of this.
Perhaps it does, but why bother? Do you propose we change the initial
size of the array every time we add an auth provider. So the
reallocation may be ugly, but it doesn't affect the behaviour of the
client in any measurable way.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org