Re: svn commit: r40218 - trunk/subversion/svn

On Sat, Oct 24, 2009 at 18:42, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> Greg Stein wrote:
>> On Sat, Oct 24, 2009 at 08:35, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>> >...
>> > +++ trunk/subversion/svn/proplist-cmd.c Sat Oct 24 05:35:12 2009        (r40218)
>> >...
>> >   else  /* operate on normal, versioned properties (not revprops) */
>> >     {
>> > -      apr_pool_t *subpool = svn_pool_create(pool);
>> > +      apr_pool_t *iterpool;
>> >       svn_proplist_receiver_t pl_receiver;
>> >
>> >       if (opt_state->xml)
>> >         {
>> > -          SVN_ERR(svn_cl__xml_print_header("properties", pool));
>> > +          SVN_ERR(svn_cl__xml_print_header("properties", scratch_pool));
>> >           pl_receiver = proplist_receiver_xml;
>> >         }
>> >       else
>> > @@ -181,6 +180,7 @@ svn_cl__proplist(apr_getopt_t *os,
>> >       if (opt_state->depth == svn_depth_unknown)
>> >         opt_state->depth = svn_depth_empty;
>> >
>> > +      iterpool = svn_pool_create(scratch_pool);
>> >       for (i = 0; i < targets->nelts; i++)
>>
>> If you construct the iterpool in the declaration (as before), then you
>> can use it for the call to svn_cl__xml_print_header(), as its
>> scratch_pool. Any mem used by the call will be cleared on the first
>> iteration of the loop.
>>
>> >...
>> > @@ -204,17 +204,16 @@ svn_cl__proplist(apr_getopt_t *os,
>> >                                         opt_state->depth,
>> >                                         opt_state->changelists,
>> >                                         pl_receiver, &pl_baton,
>> > -                                        ctx, subpool),
>> > +                                        ctx, iterpool),
>> >                    NULL, opt_state->quiet,
>> >                    SVN_ERR_UNVERSIONED_RESOURCE,
>> >                    SVN_ERR_ENTRY_NOT_FOUND,
>> >                    SVN_NO_ERROR));
>> >         }
>> > +      svn_pool_destroy(iterpool);
>> >
>> >       if (opt_state->xml)
>> > -        SVN_ERR(svn_cl__xml_print_footer("properties", pool));
>> > -
>> > -      svn_pool_destroy(subpool);
>> > +        SVN_ERR(svn_cl__xml_print_footer("properties", scratch_pool));
>> >     }
>> >
>> >   return SVN_NO_ERROR;
>>
>> And if you don't destroy it so soon, then you get to use it for that
>> last call, too.
> [...]
>
> That's true, there is the opportunity to do that, but is it really worth
> breaking the well-defined "iterpool" pattern for such a little extra
> optimisation? I don't think so. Keep it simple.

Eh? How does it break it?

You alloc the iterpool. clear it at the start of each iteration.
destroy it later. I see no change to that.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411124
Received on 2009-10-25 04:20:05 CET