[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1412418 - in /subversion/trunk/subversion: include/private/svn_string_private.h libsvn_subr/string.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 22 Nov 2012 17:52:05 +0000 (GMT)

Branko Čibej wrote:

> On 22.11.2012 16:08, Julian Foad wrote:
>> Branko Čibej wrote:
>>> +/** A self-contained memory buffer of known size.
>>> + *
>>> + * Intended to be used where a single variable-sized buffer is needed
>>> + * within an iteration, a scratch pool is available and we want to
>>> + * avoid the cost of creating another pool just for the iteration.
>>> + */
>>> +typedef struct svn_membuf_t
>> What does that comment about scratch pools and iterpools mean?
>>
>> I don't follow what characteristics this new memory buffer has, and
>> how it compares with svn_stringbuf_t.  You realize svn_stringbuf_t
>> already supports arbitrary data with null characters in it?
>>
>> svn_stringbuf_t seems to be all of this, and more because it tracks
>> the length of the user's data separately from the allocated length.
>> What are the advantages of this new buffer?
>
> Imagine the case of this LCS computation, that needs to allocate some
> memory for the algorithm. Typically we'd do that by passing a scratch
> pool to the function and let it allocate from that. And if the caller
> did this in a loop, it would create an iterpool, which would get cleared
> in every iteration.
>
> This is fine as long as the buffer allocation and pool clearing are
> cheap relative to the rest of the loop, and if the overhead of creating
> a pool just for that isn't a problem. But there are cases where this
> simply does not hold, and it's nice to be able to use the same buffer
> over and over again, without having to reallocate it every time.

OK, I follow that, but you're basically just saying "this buffer type is designed be
re-used efficiently".  The mention of loops and scratch pools has little to do with the buffer
concept itself.

> I have a similar situation on the wc-collate-path branch (in the key
> comparator), for which I used stringbufs. But I ran into a few snags:
>
>   * Stringbufs always allocate the extra \0

Why is that a snag?

>   * Resizing a stringbuf will always copy its contents

Depends what you mean by "resizing".  It only re-allocs memory and copies the contents when the required "user data size" surpasses the allocated size, and then it doubles the allocation so it won't do it too often.  stringbuf_setempty() followed by stringbuf_ensure(whatever) won't copy anything.

>   * Stringbuf's data pointer is a char*

So what?  The inconvenience of casting to/from (something_else *)?

> I solved the second by adding an svn_stringbuf__resize private function,
> but couldn't really solve the first without completely reimplementing
> stringbufs. The third required a bunch of yucky casts.

I don't get it.  (Haven't looked at your __resize() yet.)

> My intention is to try to share implementation between stringbufs and
> membufs. If that doesn't work, I'll probably just throw these membufs
> away; but I'm hoping that won't be necessary. Another alternative is to
> turn this around and make the membuf API a wrapper around stringbufs;
> but that seems just wrong given that membufs are the more generic concept.

Cool.

- Julian
Received on 2012-11-22 18:52:43 CET

This is an archived mail posted to the Subversion Dev mailing list.