Evgeny Kotkov wrote:
> Stefan Fuhrmann <stef...@apache.org> writes:
>> +static svn_error_t *
>> +get_phys_change_count(query_t *query,
>> + revision_info_t *revision_info,
>> + apr_pool_t *scratch_pool)
>> {
>> + apr_pool_t *subpool = svn_pool_create(scratch_pool);
[...]
>> + svn_pool_destroy(subpool);
>> + return SVN_NO_ERROR;
>> }
>
> What would be the reason to create a subpool here? AFAIK, this is considered
> to be a bad practice, and there is a corresponding statement in the Subversion
> Community Guide [1]:
>
> Functions should not create/destroy pools for their operation; they should
> use a pool provided by the caller. Again, the caller knows more about how
> the
> function will be used, how often, how many times, etc. thus, it should be in
> charge of the function's memory usage.
>
> For example, the caller might know that the app will exit upon the
> function's
> return. Thus, the function would create extra work if it built/destroyed a
> pool. Instead, it should use the passed-in pool, which the caller is going
> to be tossing as part of app-exit anyway.
That "example" is spectacularly stupid -- the overhead would be insignificant.
Perhaps a better example is that of a caller calling the function many times in
a tight loop, clearing the scratch pool on each iteration. Then the creation of
an internal subpool is unnecessary and could be a significant overhead. Should
we use that example instead in the community guide?
No comment on whether a sub-pool is useful in the particular cases you've
noticed.
- Julian
