On Thu, Sep 23, 2010 at 2:20 PM, Branko Čibej <brane_at_xbc.nu> wrote:
> On 22.09.2010 21:41, Hyrum K. Wright wrote:
>> [ apologizes for the somewhat stream-of-conscious nature of these mails ]
>>
>> On Wed, Sep 22, 2010 at 7:16 PM, Hyrum K. Wright
>> <hyrum_wright_at_mail.utexas.edu> wrote:
>>> On Wed, Sep 22, 2010 at 5:35 PM, Hyrum K. Wright
>>> <hyrum_wright_at_mail.utexas.edu> wrote:
>>>> For the C++ folks out there, I've got a question about an approach to
>>>> take on the object-model branch. At issue is how to wrap the various
>>>> C structures returned to callers, particularly in a backward
>>>> compatible manner. Currently, I'm looking at svn_wc_notify_t *. As I
>>>> see it, there are a few options:
>>>>
>>>> 1) Just wrap the pointer to the C struct as a member of the wrapper class.
>>>> Pros: Easy to implement; lightweight constructor.
>>>> Cons: Getters would need to translate to C++ types; would need to
>>>> implement a copy constructor which deep copies the C struct; would
>>>> also introduce pools, since creating and duplicating C structs
>>>> requires them.
>>>>
>>>> 2) Wrap each C struct member individually
>>>> Pros: C->C++ complexity is constrained to the constructor,
>>>> everything else is C++ types
>>>> Cons: Hard to extend for future compatibility
>>>>
>>>> 3) Just pass the C-struct pointer around; don't even bother with a class
>>>> Pros: Dead simple.
>>>> Cons: Requires more memory management thought by consumers; not
>>>> C++-y enough; may introduce wrapping difficulties.
>>>>
>>>> I'd like to come up with something consistent, which would be used
>>>> throughout the C++ bindings. I'm also interested in a solution which
>>>> ensures the C++ bindings can be used as the basis for other
>>>> object-oriented bindings models (Python, Perl, etc.)
>>> After lunch, and some thought, it feels like #1 is the best solution.
>>> This doesn't change the external class interface, which is good, and
>>> can still provide C++ values to callers who want them. The pool
>>> issues are a bit messy, but at least the object can manage it's own
>>> memory (albeit at a significant overhead).
>> This could get ugly.
>>
>> Creating and destroying pools all over the place could get ugly, but
>> it's necessary evil because all of our object creation / duplication
>> functions all require a pool. An alternative would be a set of
>> functions returning the size of the object, and then another which
>> puts the object in a pre-allocated memory location. (These could
>> theoretically replace the pool argument version of the API, but that'd
>> be *a lot* of churn.)
>>
>> The approach would let the C++ allocate the memory (of the correct
>> size) using whatever scheme it wants, and then do a "placement
>> initialize" using the second API. If we do go this route, I'd
>> recommend exposing these as private-to-Subversion, at least initially.
>>
>> The other option is just pass the C-struct pointer around everywhere,
>> but then the bindings consumers have to work about this exact same
>> issue. In other words, it solves it now, but really just pushes the
>> problem elsewhere.
>>
>> -Hyrum
>
> Memory management with pools and C++ -- keep away from doing it in
> per-object ctor/dtor pairs is all I can say. Lifetimes would get so
> messy and mucked up you could hardly believe it.
>
> IMHO the best way to wrap the C structures in C++ is to subclass 'em.
>
> class svn_some_thing : public svn_some_thing_t { .... };
>
> This way you can pass C++ pointers directly to the C implementation,
> most methods become just inlined wrappers. Callbacks are a bit more
> hairy, but not all that much.
This works for input types, but right now it's the output types (which
are much more common) that I'm concerned about. Things like
svn_commit_info_t, or svn_wc_notify_t. For my initial hack at it see:
http://svn.apache.org/repos/asf/subversion/branches/object-model/subversion/bindings/c++/include/Types.h?p=1000600
> As to pools ... I'd once thought that a proper C++ wrapper for a pool
> would be a reference-counted smart pointer. That turns out to be too
> much overhead and too much of a good thing, since you *cannot* allow
> pool lifetime to depend on some reference counting order of execution.
> Best just use plain pool pointers, or maybe wrap them minimally so that
> destructors do the pool cleanup reliably.
Yeah, we've got a Pool class which destroys the underlying APR pool in
the dtor, so that provides a handy way to clean things up (if it
weren't for the 8k overhead of the minimum pool size). To the initial
hack, I've added a smart pointer to prevent multiple redundant copies
of the underlying C struct, since I expect it to be read-only (we
could probably add an explicit Clone() method if folks really wanted a
deep copy). The smart pointer magic is here:
http://svn.apache.org/repos/asf/subversion/branches/object-model/subversion/bindings/c++/include/Types.h
There's still a self-managed pool for allocating the C-struct, but
it's encapsulated enough to be changable should we have a better way
of duplicating objects in the future.
Thanks for the insight, by the way. These classes and paradigms are
in no way set in stone, and I welcome discussion. I sense you've got
a bit more C++ design experience than I do. :)
-Hyrum
Received on 2010-09-24 04:05:55 CEST