Compiling SWIG python bindings with -builtin option

Hello,

I use the vector_source_x and vector_sinc_x blocks quite a lot for
testing, debugging and simulations, even with quite large input and
output data streams.

Therefore I was looking in speeding up the feeding and retrieval of data
by implementing the Python buffer interface [0] that permits to exposed
data to other components handling uniform vectors of data in a copy-free
manner.

Investigating how to do that in SWIG I found that [1] the only supported
way of doing this is to compile the bindings with the SWIG -builtin
option [2] that modifies how the Python bindings are constructed. Making
a long story short, the Python class definition is not done anymore in
python code, but with a proper Python extension (in a more clean way,
IMHO).

At first I tried to implement this in an out-of-tree GNURadio module,
however I run into problems because SWIG is unable to generate bindings
with the -builtin flag for types subclassing from types defined in an
extension compiled without the -builtin option (sorry, that reads a bit
too convoluted…).

There is interest in switching GNURadio over to the -builtin way of
generating the bindings? We would gain somehow cleaner bindings, the
possibility of populating the Python objects slots to implement specific
object behaviors (to get rid of the repr() override necessary for
each block, and the buffer interface, for example).

If there is interest, I can look into providing a patch. A quick test
revealed no problems in adding the -builtin option to the makefiles.

Thank you for reading all that :slight_smile:

Cheers,
Daniele

[0] Buffers and Memoryview Objects — Python 2.7.18 documentation
[1]
http://swig.10945.n7.nabble.com/swig-python-extension-accessing-PyObject-to-implment-pep-3118-revised-buffer-Protocol-td2419.html
[2]
http://quantlib.org/reposit/docs/swig/Python.html#Python_builtin_types

Daniele,

this is interesting. However, I’m not 100% clear about the implications
this would have (i.e. which changes are necessary). Is it just the build
flag, and then of course the accompanying limitations listed in the
manual (those would not be a problem, as far as I can tell)?

One thing I can say for sure is that it won’t be merged into GNU Radio
3.7, since it requires a compat bump for SWIG. However, we are planning
that for 3.8, so this change is not unreasonable.

This is new to me, and I just gave the manual a quick browse. From that,
it seems a good idea, but as I said, I can’t quite see all the
consequences.

If you could provide a branch with the necessary changes (an incomplete
proof of concept would be fine, just so we can see what needs to be
changed), branched off of next, that would be great.

Thanks for suggesting this!

Martin

Hello Martin,

thank you for reviewing this idea. I’m already working on a branch with
the required changes.

The required changes are the build flag and some adjustment to the SWIG
bindings. With builtin object classes there isn’t anymore a python
wrapper class therefore it is not possible to fiddle with the class
methods at runtime. For example, the repr() method cannot be patched
in at runtime but needs to be defined in the SWIG interface.

So far I have gnuradio-runtime, including pmt, and gr-blocks compiling
and I haven’t found any major change required and I’m looking for the
cleaner way to implement some things. I haven’t yet tried to propagate
my changes to the other modules.

I hope to have a proof of concept ready in the next few days. I’m not
familiar with SWIG (and it is not very friendly), so the process of
finding the right way of doing things has been mostly a process of try
and error.

Cheers,
Daniele

On 20/10/14 11:50, Martin B. wrote:

in at runtime but needs to be defined in the SWIG interface.

Oh, we know what you’re talking about :slight_smile:

I’m interested what happens when you reach blocks that do fiddle with
SWIG. gr-uhd does this, and in gr-digital with have a couple of tweaks.

I had a look at what gr-uhd does in its python code and it looks like it
mostly corrects for things that could be also solved easily in SWIG.
Worst case what can be done is that the SWIGGed class is renamed (with a
SWIG directive) and at is sub-classed in Python to tweat with it at
will. However, I don’t see the need of it so far.

Having to write .i files for everything would be a nuisance, given that
we mostly got rid of that in 3.7.

I don’t understand what you mean with this. We have .i files for
everything! For out-of-tree modules they are auto-generated by
gr_modtool and some parts are hidden behind SWIG macros, but there
definitely are .i files for all the classes in GNURadio. It is how SWIG
works :slight_smile:

Also, I’m very interested in benchmarks. If there’s some effort involved
here, it has to pay off in terms of speed.

I don’t think there are any speed advantages for blocks coded in C++,
what can be sped up is instantiation and configuration of the blocks.
The actual work methods are already called in C++ context without Python
overhead, so there is nothing to to gain there.

This is different for blocks coded in Python, but to take advantage of
the improved SWIG wrappers other changes would be required, probably.
There is quite a bit of magic involved at the moment, that can be
probably simplified with better wrappers.

However, I have the feeling that if speed is a goal, SWIG is probably
not the right tool. The code it generates it sub-optimal under many
aspects (starting with the double type system it puts in place: the SWIG
type system somehow glued on top of the Python type system…)

I’d suggest you open an issue on our tracker, and we take the discussion
there. I’m hoping that that some people with more SWIG experience can
chime in here, too.

I’ll do.

Cheers,
Daniele

On 10/20/2014 12:16 PM, Daniele N. wrote:

Having to write .i files for everything would be a nuisance, given that
we mostly got rid of that in 3.7.

I don’t understand what you mean with this. We have .i files for
everything! For out-of-tree modules they are auto-generated by
gr_modtool and some parts are hidden behind SWIG macros, but there
definitely are .i files for all the classes in GNURadio. It is how SWIG
works :slight_smile:

No, we barely have any (unless you’re using a very old GNU Radio). We
have top-level .i files which include the actual block header,
per-in-tree component if that’s what you mean. We do not need to write
.i files for every block (and are glad about it :).
The few modifications we need these days are taken care of by modtool.

There is quite a bit of magic involved at the moment, that can be
probably simplified with better wrappers.

However, I have the feeling that if speed is a goal, SWIG is probably
not the right tool. The code it generates it sub-optimal under many
aspects (starting with the double type system it puts in place: the SWIG
type system somehow glued on top of the Python type system…)

SWIG isn’t going anywhere soon. But if we can improve on what we have,
that’s interesting.

I’d suggest you open an issue on our tracker, and we take the discussion
there. I’m hoping that that some people with more SWIG experience can
chime in here, too.

I’ll do.

Thanks!

M

On 20/10/14 13:46, Martin B. wrote:

No, we barely have any (unless you’re using a very old GNU Radio). We
have top-level .i files which include the actual block header,
per-in-tree component if that’s what you mean. We do not need to write
.i files for every block (and are glad about it :).
The few modifications we need these days are taken care of by modtool.

Now I see what you mean, but the fact that the .i files are quite small
and mostly auto-generated does not change the fact that we need the .i
files to tell SWIG to generate code for the blocks :slight_smile:

The .i files for regular GNURadio blocks are not going to change much
with the switch to builtin object classes. The only changes required are
related to providing a repr() method because, unfortunately, the
glued on swig type system does not make it possible to have it inherited
from parent classes, at least as far as my understand of SWIG goes.

I’ll try to have a branch with my changes ready to review soon, so we
can talk about something more concrete.

Cheers,
Daniele

On 10/20/2014 11:19 AM, Daniele N. wrote:

So far I have gnuradio-runtime, including pmt, and gr-blocks compiling
and I haven’t found any major change required and I’m looking for the
cleaner way to implement some things. I haven’t yet tried to propagate
my changes to the other modules.

I hope to have a proof of concept ready in the next few days. I’m not
familiar with SWIG (and it is not very friendly), so the process of
finding the right way of doing things has been mostly a process of try
and error.

Oh, we know what you’re talking about :slight_smile:

I’m interested what happens when you reach blocks that do fiddle with
SWIG. gr-uhd does this, and in gr-digital with have a couple of tweaks.
Having to write .i files for everything would be a nuisance, given that
we mostly got rid of that in 3.7.

Also, I’m very interested in benchmarks. If there’s some effort involved
here, it has to pay off in terms of speed.

I’d suggest you open an issue on our tracker, and we take the discussion
there. I’m hoping that that some people with more SWIG experience can
chime in here, too.

M


Discuss-gnuradio mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/discuss-gnuradio

On 10/28/2014 12:58 AM, Daniele N. wrote:

I prefer to spent my spare time doing something more rewarding that
fighting an under documented, inconsistent, and buggy system.

Thanks for trying. Can’t blame you for being fed up with SWIG.

M

On 20/10/14 14:06, Daniele N. wrote:

I’ll try to have a branch with my changes ready to review soon, so we
can talk about something more concrete.

Hello,

a short update on the progress of my branch. So far I haven’t
encountered major problems (the progress is limited by my limited free
time and the long compile times). However, I have hit what seems to be a
major limitation / bug of SWIG.

It seems impossible to use both the builtin and keyword options when
wrapping std::vector template classes. Using the keyword option with
std::vector spits out a number of warning, but produces code that
compiles, adding the builtin option the code does not compile anymore.

At the beginning I simply dropped the keyword option, but now I hit some
tests that require methods wrapped with it, so I’m investigation what
may be a possible solution / workaround.

Cheers,
Daniele

Hello,

This is to inform you that I’m giving up trying to transition the swig
bindings to generate builtin classes. I may revisit this decision if
I’ll get encouraging answers from swig developers on the swig mailing
lists, but I believe this is very unlikely.

While the changes to the interface definition files are nominally
extremely small, I’m hitting bugs in swig that make the exercise
extremely painful and I fear impossible without patching swig itself.
I also fear that no matter what I can do, the binding egenrated are
going to stay sub-optimal due to intrinsic limitations of swig.

I prefer to spent my spare time doing something more rewarding that
fighting an under documented, inconsistent, and buggy system.

Cheers,
Daniele

On 28/10/14 11:54, Martin B. wrote:

going to stay sub-optimal due to intrinsic limitations of swig.

I prefer to spent my spare time doing something more rewarding that
fighting an under documented, inconsistent, and buggy system.

Thanks for trying. Can’t blame you for being fed up with SWIG.

It is not that I’m fed up, it is simply that SWIG cannot do what I would
like it to do. To help others that may venture down the same route, I
think the decision has to be better circumstanced.

The first road block I encountered is the fact that there is a bug in
SWIG that prevents to mix the -builtin (to enable the generation of
nicer python builtin objects instead of the usual abomination) and the
-keyword parameter (to enable calling the generated methods with keyword
arguments) when wrapping std::vector template instances. The generated
C++ simply does not compile.

Part of the problem comes from the fact that the C++ code SWIG generates
to wrap std::vector uses overloaded methods, and SWIG refuses to enable
keywords arguments for those. From there things are very brittle and
breaks in numerous ways.

By accident I found out that keyword arguments can be enabled and
disabled for specific classes (this is AFAIK undocumented), therefore
the problem could be in principle be circumvented adding some additional
SWIG directives to the interface description files.

The second road block is related to the first: the make() method of many
GNURadio blocks is an overloaded method, and for those SWIG refuses to
enable keywords arguments. For some reason keywords arguments work
anyway when bindings are generated without the -builtin option, but do
not work with it. Looking at the generated C++ code, it seems that
keywords arguments are enabled in some layers of the wrapping code, but
disabled where it matters.

A solution would be to give up the use of keyword arguments to method
calls, but I think this would be loosing a nice feature, just for a
limitation of a stupid tool.

While fixing SWIG is probably non trivial but possible, I don’t think
that requiring a very recent SWIG would be accepted by the maintainers.
Therefore I decided to give up, the advantages are not worth fighting.

I prefer investing some times to experiment with the idea of generating
Cython wrappers and drop SWIG completely.

Cheers,
Daniele