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

Hi Jake,

A lot to think about there :)

On your first bits:

> - 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.
Yes extra memory usage is not ideal, but my thinking is that there would 
not actually be many SwigPyObject objects in existence at anyone time in 
typical usage. Certainly in my usage most of the data manipulation goes 
on in the C++ library and the Python just accesses the data items. Other 
usages cases could be different though I am mainly thinking from my 
usage cases and don't know all of the things SWIG is used for.


> - 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.

For the member access case the actual SWIG wrapper generation code only 
generates the SwigContainerReference() calls in the situation where a 
member is returned and is not used in the case of a function return so 
that limits its general overhead somewhat. On the return of basic 'C' 
member types by value, the SWIG C++ parser has a lot of info on the type 
in order to generate the SWIG_From_int() and other such calls, so it 
should be fairly easy to not insert the SwigContainerReference() in 
these cases for better efficiency when swig generated the wrapper if 
wanted. Not sure what the actual overhead of a Py_INCREF() call is ?

For the more complex situation of dealing with STL container classes and 
accessing container entries with the generic template value_type, I 
haven't looked into how the STL value_type works, but I guess the traits 
system could still be used to decide if reference counting is needed to 
improve performance in the specific %template functions needed for 
these. I guess with a lot more work SWIG could handle templates at the 
wrapper generation stage to a degree and determine the fact that a 
reference is being returned when it generates the wrapper.


On using %feature vs %typemap, from what I can see, and maybe I am 
wrong, using reference counting whenever a reference/point to an objects 
member is directly returned by SWIG, would be a normal and standard 
requirement. Hence the use of %feature rather than %typemap.

> 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.
Thanks, I don't know the SWIG code :)

> Many languages are not reference counted, they use graph reachability 
> with periodic GC.
Yes, in these cases maybe the SwigContainerReference(), 
SwigContainerDeReference() and the object pointer could be used as 
needed to achieve the necessary lifetime management. For myself we are 
only interested in Python ATM :)


> Type trait check should be kept for efficiency of int, string, etc, 
> but implementation should switch to the refcount.
For the member access I don't think the Type trait is needed (?), but 
for the STL return of a container entry situation, personally, I think 
it would be nice to simplify and standardise the lifetime management 
some how, losing the fragment and complex traits C++ syntax. Maybe 
something like a SwigContainerReferenceIfNeeded($*l_type, $self, 
$result) macro could be added to pwrun.swig for more general usage in 
%typemaps ?

But as stated I'm not really up with SWIG internals or its overall usage 
situations.


Terry

On 14/08/2020 23:33, Jake 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] 
> <mailto:[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