+1, this is a good memory management strategy.
(The "don't keep a pool in a baton" thing is already in HACKING; "use
two pools for new code or code you can adjust without compatibility
issues" could definitely go there too.)
--dave
On Mon, Oct 6, 2008 at 12:02 AM, Greg Stein <gstein_at_gmail.com> wrote:
> Hey all,
>
> On IRC, danielsh asked where I got the convention for
> result_pool/scratch_pool that I've been using in wc_db.h and when I've
> been adding more interfaces elsewhere. I wasn't on IRC at the time to
> answer, and figured the mailing list would be a good place to do that
> so others can read, too.
>
> Way, WAY back when, I recall an FS function taking two pool arguments.
> One for results, and one for temporary stuff. I don't remember why or
> which function, but I think that sparked the idea. Over time, there
> have been situations where it has become obvious that two conceptual
> pools should be presented to a function: one for the results because
> they will need to stick around beyond the lifetime of the function
> call, and a second for any scratch/temporary work that the function
> needs to perform. This is *especially* noticeable if the function does
> a LOT of temporary allocations (i.e. calling into subsystems).
>
> You can find the latter in our codebase: a function will allocate a
> subpool, do its work, place results in the provided pool, then destroy
> the subpool on exit. This is bad.
>
> Why is it bad? The caller knows *much* more about what is going on
> that the function itself does. The caller may be in an iterative loop
> and already has a subpool for scratch allocations, so creating a new
> pool is just stupid overhead. Or the caller knows that it is "done",
> so any allocation can just go into a specific pool because there is no
> need for destroying a pool to "make room" for something else --
> everything will be destroyed in just a moment. The caller may be
> provided a scratch pool and pass that into a sub-call because sharing
> the same scratch pool keeps the same "high water" mark on memory
> allocation(*).
>
> In short, whenever you see a pool creation for something *other* than
> an iterative pool, then it is bad. Some function signature(s)
> somewhere need to be adjusted.
>
> Along different lines, if an "object" stores a pool into itself for
> later use (we're talking an object, not a baton which is just a
> manifestation of control flow), then it is most probably also bad. Any
> allocations should be under the control of callers. In general, the
> caller should provide a result_pool with the same/longer lifetime as
> the object, rather than allow the object to hold a pool for future
> allocations.
>
> Note that these patterns of pool use were essentially found through
> the development of Subversion. The Apache httpd server never developed
> a rigorous pattern of pool use because it pretty much uses just a few
> pools during the processing of any given request. In a few key areas,
> an iteration pool is used, but nobody recognized that as a formal
> pattern of pool use. Within Subversion, we "discovered" that proper
> use of pools across a complex set of libraries and objects was a
> requirement, and developed these patterns to meet that need. When I
> started the wc_db.h interface, I used the result_pool/scratch_pool
> convention with an intent to add that to our standard pattern
> throughout our system. It is a convention that improves on our use of
> pools. If nobody has any object to the pattern, then i'd like to
> document it in hacking.html under pool use and also mention the
> convention for function signatures.
>
> Thoughts?
>
> Cheers,
> -g
>
> (*) there is one case where these self-allocated pools keep the
> high-water mark low: if you call two functions in series, each using a
> self-pool, then the high water mark is the max of each function,
> rather than the sum. avoiding the sum would be important if a function
> knows it uses a crazy amount of memory. I would maintain the need for
> a self-pool is rare, and we just state the convention is to avoid, but
> document/comment on a case-by-case basis when it is needed/used.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-07 00:50:46 CEST