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

pool usage (was: Re: CVS update: subversion/subversion/libsvn_fs rev-table.c)

From: Greg Stein <gstein_at_lyra.org>
Date: 2001-03-09 13:31:19 CET

On Tue, Mar 06, 2001 at 12:42:47PM -0500, Greg Hudson wrote:
> Ben wrote:
>
> > Note that svn_stream_close() does not deallocate the memory
> > associated with the stream; destroy the pool you created the
> > stream in to do that.
>
> > So in GregH's last sentence, what does "memory associated with the
> > stream" mean?
>
> Sorry to be unclear. I meant the memory allocated for the stream
> structure itself. I was just trying to say that, per Greg Stein's
> discipline, stream structures are not allocated in a subpool. No
> memory is allocated by the stream abstraction in the process of
> accessing the stream.

To clarify, I recommend one of two approaches:

1) allocate the object in the provided pool
2) create a subpool, alloc the object in the subpool

Now, that GregH is saying is that (2) shouldn't be used in this case because
the subpool is never used later on. The corollary is that a subpool in this
case would be a fine-grained alloc/free mechanism.

If the allocation is going to be large, then a subpool could be nice. When
somebody calls svn_stream_close() (*if* they do so), then we can get rid of
a big chunk of memory for them. If they never call it, then it'll go away
when they toss their pool.

I believe the hardest question is how the caller should determine their own
behavior. Let's say the caller has an unbounded loop, creating streams. They
cannot simply pass the same pool to each svn_stream_create() function,
because they could end up with a billion streams in that one pool. Thus, the
guy doing the loop will create a subpool, then call svn_stream_create
passing the subpool, then clear the subpool and loop. If the create function
then decides to create a subpool, then we've now got two subpools per
iteration.

So... this means that option (1) is almost always the proper style. Defer to
your caller to let them decide how to manage memory. If they are going to
call you many times, then they may use subpools. Or they simply give you
their own.

An "object" should create a subpool for the things that *it* calls. If my
stream is going to call down into the FS for each read() call, then I'll
probably want to create a subpool at create time, pass it to the FS during
the read, then clear it before returning.

>...
> > (Of course, why would anyone *want* that kind of control? What use
> > would the stream object be if its hidden baton were destroyed? Why
> > would you even want the stream object around anymore?)
>
> It's not a question of control. It's a question of not creating
> subpools when it's not necessary. We don't create subpools for
> svn_string_t objects either, not because we want control but because
> it's simpler.

Exactly. We create too many, and as a result, our overall memory policy is
shaky and uneven. Subpools are good for unbounded loops, when you want to
clear the results of each iteration.

> The point of svn_stream_close is not to free memory. It is to say
> "I'm done {reading/writing} this stream now," which is important for
> things like svndiffs.

Exactly. Comments in the header about "freeing" usually point to a potential
problem. An abstraction should not talk about freeing its memory. The caller
gives it a pool, and that pool defines the lifetime. The object may choose
to use a subpool for internal, unbounded loops, but that is *internal*, so
it wouldn't show up in the API docs.

> Our pool management system means that an
> explicit destruction method is not necessary for every data type we
> have, but for some reason we don't seem to all be on the same page
> about that.

Hmm. Maybe this stuff can go into the HACKING document?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:25 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.