Re: C++ to Python and returning members of a C++ object

For the mailing list I wasn't able to include a zip attachment, so the
basis of my testing is this:

struct MyObject {
MyObject() = default;
MyObject(std::string s) : s(std::move(s)) { }
std::string s = "MyObject";
};

struct Mixed {
MyObject obj;
int i;
int& getI() { return i; }
const int& getIConst() const { return i; }
MyObject& getObject() { return obj; }
const MyObject& getObjectConst() const { return obj; }
};


On Fri, Aug 14, 2020 at 6:33 PM Jake <[email protected]> wrote:

> First a TLDR:
>
> - Back-ref costs 46% more CPU than your ref count scheme; ref count scheme
> costs 8 bytes memory per swig-wrapped object even when not needed.
> - Dropping the traits check adds run-time cost to cases where counting is
> not needed.  Run-time overhead is about 12%, traits check has no overhead
> in these cases.
> - Traits and your ref count scheme could be combined, but the traits are
> difficult to figure out.  Maybe we have to eat the run-time check cost for
> member access and member functions in the more general case.
>
>
> On Wed, Aug 12, 2020 at 8:24 AM Terry Barnaby <[email protected]> wrote:
>
>> Hi Jake,
>>
>> Thanks for the info. I have spent a bit of time trying to understand how
>> that typemap with "reference_container_owner" fragment and swig::traits
>> works and have tried to implement a referenced counted class member return
>> system with it. Now I may be being stupid here but what I see is:
>>
>> 1. The %typemap ret or out is used in all cases when a value/object is
>> "returned" from C++ to Python. So it is used not only when an objects
>> member is returned by reference/pointer but also when a C++ function
>> returns a value/object by reference/pointer. To handle the case of
>> incrementing the reference of a container object only when one of its data
>> members is returned something like a %typemap memberout and/or memberret
>> needs to be implemented so that it is only used when returning data members
>> by reference/pointer.
>>
>
> Yes, for the member case I think a memberret typemap support would be
> best.  The reason the container patch uses 'ret' and not 'out' is because
> we don't want to change how the value gets handled, just to add on the
> extra code after whatever already happens due to 'out'.  The container
> patch is fine using 'ret' because the placement in the body of the
> container limits the scope where it matches so it will not trigger on free
> functions elsewhere.
>
> 2. The system uses the "traits" system to, at runtime, determine if the
>> return is a reference or not for all functions returning that type.
>>
>
> The type traits are template specialization, so it is a compile-time check
> not a run-time check.  If the trait is swig::value_category it calls an
> empty function which any compiler will just remove, otherwise for
> swig::pointer_category the back-reference kicks in e.g. in std::vector<Foo>
> we expect swig::traits<Foo>::category to be swig::pointer_category, but in
> std::vector<int> we expect swig::traits<int>::category to be
> swig::value_category.  At least for containers, when you have "int&
> operator[](size_t index)" and in python you say "v = container[0]" it
> copies the int, so ref counting the container is not desired.
>
> Your approach does a run-time check always, which I also started with but
> it was complained about in the PR:
> https://github.com/swig/swig/pull/1234#issuecomment-453778142
>
> Have a look at the test I attached.  If you look at the generated member
> out code for Mixed::i, it ends like this:
>
>   arg1 = *reinterpret_cast*< Mixed * >(argp1);
>
>   result = (*int*) ((arg1)->i);
>
>   resultobj = SWIG_From_int(*static_cast*< *int* >(result));
>
>   *return* resultobj;
>
> So the int is a copy, not a pointer.  The same thing happens for
> MyObject::s, which generates:
>
>   arg1 = *reinterpret_cast*< MyObject * >(argp1);
>
>   result = (std::string *) & ((arg1)->s);
>
>   resultobj = SWIG_From_std_string(*static_cast*< std::string >(*result));
>
> But compare that to the int& freeInt() function:
>
>   result = (*int* *) &freeInt();
>
>   resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), SWIGTYPE_p_int,
> 0 |  0 );
>
> Of course we don't want to back-ref this one anyway but I just want to
> show it's wrapped as a pointer not a value.
>
> This is a very tricky part of SWIG, but if we could figure it out between
> us then maybe we could have a general solution for containers, member
> accesses, and member functions returning non-owned pointers generally.  Or
> else it should be agreed that the runtime check of SWIG_Python_GetSwigThis
> on every access is worth avoiding the complexity of figuring out when it's
> needed based on type (or because there is no fully general type-based
> solution).  Here is why it's a pain though:
>
> Primitive types get their swig::value_category traits from
> Lib/python/std_common.i, which kicks it up to Lib/typemaps/primtypes.swg.
> Other types I haven't been able to figure out where they come from.
> However, the problem is the traits are not defined by default.  In the
> attached test code, comment out this line:
>
>  %template(StringVector) std::vector<std::string>;
>
> And you will get compiler errors because
> swig::traits<std::string>::category is not defined.  Same thing for int and
> our custom type MyObject.  Somehow the machinery for container support
> drives these trait definitions into existence when they get used as the
> value_type for a container, but I have not been able to figure out how.
> They do seem to do the right thing though, with std::string getting
> swig::value_category and MyObject getting swig::pointer_category.  Could we
> invoke this machinery somehow from a typemap to make sure the traits we
> need to check are always defined?
>
> There is another complication because int& generates wrapped pointer code
> and const int& generates copy code, e.g. comparing Mixed::getI to
> Mixed::getIConst:
>
>   arg1 = *reinterpret_cast*< Mixed * >(argp1);
>
>   result = (*int* *) &(arg1)->getI();
>
>   resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), SWIGTYPE_p_int,
> 0 |  0 );
>
> vs
>
>   arg1 = *reinterpret_cast*< Mixed * >(argp1);
>
>   result = (*int* *) &((Mixed *const* *)arg1)->getIConst();
>
>   resultobj = SWIG_From_int(*static_cast*< *int* >(*result));
>
> But SWIG's traits system doesn't consider const-vs-non-const so either the
> typemap itself or another mechanism akin to std::is_const would be used.
> Containers seems to always use the const version so we get away with not
> making the distinction.
>
> Last issue is what happens if a user-defined typemap overrides the normal
> conversion behavior.  Typemaps as opposed to a %feature let's this be
> overridden on case-by-case basis if needed.  Downside is they could
> accidently turn off the protection on a case-by-case basis where the
> %feature is always inserting it on top of the active typemap.  An argument
> for the run-time check is we don't have to consider this at all, you just
> check whatever the typemap converts at run-time and then ref-count or not.
>
>
>> 3. The system adds a pointer to the container within the Python object
>> using Python functionality. This appears to be quite cumbersome/inefficient
>> to me for such a basic need (reference counting of used objects).
>>
>
> Your approach avoids a call to PyObject_SetAttr (some error checking, a
> call through a function pointer, a __dict__ insertion), adds a couple
> pointer checks on dereference, but increases the size of every wrapped
> object by one pointer regardless of use.  Memory for CPU efficiency seems a
> worthwhile tradeoff for common workloads.  Of course without a compile-time
> way to check whether the counting is needed you have extra overhead on
> value-types, a systematic efficiency hit.
>
> I did some benchmarking for 100 million accesses in a loop.  First here is
> the int& case where counting is not needed.  Trait check is like baseline,
> swig-this check adds about 12% overhead:
>
> $ echo "Access Mixed.i baseline" && time python3 accesstest.py
> Access Mixed.i baseline
>
> real 0m11.368s
> user 0m11.072s
> sys 0m0.277s
>
> $ echo "Access Mixed.i backref trait check" && time python3 accesstest.py
> Access Mixed.i backref trait check
>
> real 0m11.586s
> user 0m11.281s
> sys 0m0.284s
>
> $ echo "Access Mixed.i refcount check" && time python3 accesstest.py
> Access Mixed.i swigthis check
>
> real 0m12.807s
> user 0m12.504s
> sys 0m0.284s
>
> Now compare your refcount scheme against setting a back reference
> attribute, the backref has about 46% overhead over the refcount:
>
> $ echo "Access Mixed.obj refcount" && time python3 pointertest.py
> Access Mixed.obj refcount
>
> real 0m15.476s
> user 0m15.152s
> sys 0m0.293s
>
> $ echo "Access Mixed.obj backref" && time python3 pointertest.py
> Access Mixed.obj backref
>
> real 0m22.655s
> user 0m22.321s
> sys 0m0.285s
>
>
>> For information I have tidied up my small patch a little so that it is
>> enabled with %feature("containerReference") either for the whole library or
>> on a class by class basis. This appears to work fine although it could have
>> issues.
>>
>
> It segfaults on pointer-category objects because you should be passing
> "self" as the first argument not "args", I corrected that for the
> benchmarks above.
>
>
>>
>> 1. This uses the swig code to determine when a reference/pointer to an
>> objects member is being returned so handles the requirement of only being
>> used when members of objects are being returned. So none of the complex
>> traits system is needed, it isn't called unless a member is being returned
>> by reference/pointer.
>>
>
> See above for runtime overhead on value types.
>
>
>>
>> 2. A C++ pointer to the container class is added to the members returned
>> SwigPyObject and the container classes reference counter is efficiently
>> incremented. When a SwigPyObject is destroyed it checks the container
>> classes reference counter and if set decrements the containers reference
>> counter. The container reference pointer and reference counting is all
>> implemented in C++ and is simple, small and efficient.
>>
>> Unless I am missing something it seems to me:
>> 1. For most languages there needs to be a reference count type of system
>> to manage the lifetime of a libraries C++ objects as most of the languages
>> that SWIG produces an API for use dynamically created/destroyed objects and
>> references to these with garbage collectors. Doing this reference
>> counting/lifetime control within the C++ SWIG area is simpler and much more
>> efficient.
>>
>
> Many languages are not reference counted, they use graph reachability with
> periodic GC.  The general strategy of assigning a back-reference I think
> will always work whether ref-counted or not but the implementation will
> still be different across languages.  E.g. to support a back-ref in a
> static language like Java or C# the back-reference field needs to get
> inserted into the proxy class for the element, we can't just tack it on
> dynamically.  Anyway many languages will not have an equivalent of
> Py_INCREF.  I'm afraid it will be a lot of work to fix across all languages.
>
>
>>
>> 2. A SWIG internal reference counted container implementation could be
>> used by %typemaps such as your use for std::vector simplifying the typemap
>> (no fragments etc.) and making this much simpler and more efficient.
>> "%typemap(ret) value_type& { SwigContainerReference($self, $result); }"
>>
>
> Type trait check should be kept for efficiency of int, string, etc, but
> implementation should switch to the refcount.
>
> -Jake
>

_______________________________________________
Swig-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/swig-user