On Tue, Jun 30, 2009 at 11:32 AM, Hyrum K. Wright<hyrum_at_hyrumwright.org> wrote:
> Providing two pools is actually a much more efficient memory strategy. The
> main idea is that the caller of a function, not the function itself, can
> better manage the temporary memory which that function needs. For instance,
> if several functions are called in succession, and they all create, use and
> destroy their own subpools, this introduces a fair amount of both space and
> runtime overhead. However, if the calling function can create one subpool,
> and let each of the called functions use it, that overhead is greatly
> reduced.
>
> We've been using this paradigm for several months within libsvn_wc, with
> good results, and as we start pushing our changes into the public API, more
> instances of scratch_pool/result_pool will arise. You can find a more
> thorough discussion of this method, and its rationale, in this thread:
> http://svn.haxx.se/dev/archive-2008-10/0268.shtml
Sounds good.
> In any case, I don't think that the limitations of our binding framework
> should drive API decisions within the core Subversion libraries.
>
> (I'm curious, though. In what way aren't scratch_pool or result_pool
> "regular" pools?)
Good point -- I think my terminology might be a bit off.
Both scratch pools and result pools have been around for a long time,
but our SWIG bindings expect that each function will only have one of
the two pools around. I guess an "regular" (old-style) pool would be a
pool that is either a scratch pool or a result pool, but not both.
>> In general, it is quite confusing and hard to maintain that our SWIG
>> bindings are trying to make general decisions about how to handle
>> function wrapping based on the names of variables in function
>> declarations. Unfortunately, the current design of our SWIG bindings
>> requires this type of trickery. In the ctypes python bindings, we
>> simply wrap functions "as is", so there is no need for any fancy rules
>> based on variable names. This type of design, I think, is much more
>> maintainable for the long term, because it allows Subversion API
>> designers to write their functions any way they want without needing
>> to think about how our fancy SWIG rules will interpret your changes.
>
> You make a good point: relying upon bindings which depend on parameter names
> to work properly is brittle, and error-prone over the long term.
> Unfortunately, I don't have any good suggestions for eliminating this
> problem.
[...]
On Tue, Jun 30, 2009 at 11:45 AM, Stefan Sperling<stsp_at_elego.de> wrote:
> If SWIG cannot keep up with the requirements we have for our API,
> then maybe we should part from SWIG in the long term and use binding
> mechanisms such as ctypes instead? Perl and Ruby should have similar
> mechanisms to interface with C. This is an even larger amount of work
> though...
To be clear, the issue here is not with SWIG per-se. The issue is that
our Subversion SWIG bindings are very ambitious and try to do fancy
conversions just based on variable name information. It is actually
significantly easier to just leave the variables alone and not convert
them. SWIG actually leaves variables alone by default, so if we wanted
to just leave variables alone we could have significantly simpler
bindings. The big issue with simplifying our bindings, though, is
whether we are willing to completely break compatibility with all
existing users of the bindings.
>> You could also teach all three sets of bindings to understand the
>> difference between result pools and scratch pools, but that would be a
>> much bigger project.
>
> I think this will be necessary...
OK. This might not be that hard.
To fix this, we would need to change all three sets of bindings to
associate the "result_pool" with any variables that are returned by
the function.
Perl bindings, Ruby bindings: Just add "result_pool" to the list of
variables in the %apply statement. The "scratch_pool" should not be
added to the apply statement, because we don't want the perl bindings
to associate the returned variables with the scratch_pool. It would
look like this:
#if def SWIGPERL
%apply apr_pool_t *pool {
apr_pool_t *dir_pool,
apr_pool_t *file_pool,
apr_pool_t *node_pool,
apr_pool_t *result_pool
};
#endif
#if def SWIGRUBY
%apply apr_pool_t *pool {
apr_pool_t *dir_pool,
apr_pool_t *file_pool,
apr_pool_t *node_pool,
apr_pool_t *result_pool
};
#endif
Python bindings: Create a specialized typedef for scratch_pool, but
leave result_pool alone so that it will be caught by the existing
wrapper for apr_pool_t *. In this case, the important part about
wrapping scratch_pool separately is that we make sure that the
existing wrapper for apr_pool_t * does not grab the scratch pool and
think that it is being used to store variables. We would also want to
write a test case to make sure that the returned variables are
associated with the right pool.
All of the above changes should be relatively straightforward work
(maybe only a few hours) but it may be hard to find volunteers.
Cheers,
David
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2366816
Received on 2009-06-30 21:11:23 CEST