Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler warning about parameters with restrict #28

Open
xqms opened this issue Dec 9, 2020 · 4 comments
Open

Compiler warning about parameters with restrict #28

xqms opened this issue Dec 9, 2020 · 4 comments

Comments

@xqms
Copy link

xqms commented Dec 9, 2020

Hey, thanks for this awesome library! I've been using it for some time now inside my ROS package https://github.com/AIS-Bonn/nimbro_network/tree/develop.

Since updating to gcc 9.3 I'm getting a compiler warning:

contrib/wirehair/WirehairCodec.cpp: In member function ‘void wirehair::Codec::BackSubstituteAboveDiagonal()’:
contrib/wirehair/WirehairCodec.cpp:2303:39: warning: passing argument 1 to restrict-qualified parameter aliases with argument 2 [-Wrestrict]
 2303 |                         gf256_div_mem(src, src, code_value, _block_bytes);
      |                                       ^~~  ~~~
contrib/wirehair/WirehairCodec.cpp:2387:35: warning: passing argument 1 to restrict-qualified parameter aliases with argument 2 [-Wrestrict]
 2387 |                     gf256_div_mem(src, src, code_value, _block_bytes);
      |                                   ^~~  ~~~
contrib/wirehair/WirehairCodec.cpp:2652:31: warning: passing argument 1 to restrict-qualified parameter aliases with argument 2 [-Wrestrict]
 2652 |                 gf256_div_mem(src, src, code_value, _block_bytes);
      |

I guess these should be fixed since the compiler can perform optimizations assuming that the arguments are not identical. Either we should remove the restrict qualifier or modify the calling code and copy the array before doing the operation.

@catid
Copy link
Owner

catid commented Jan 1, 2021

Yeah that's a good point and I've been lazy about fixing it because it hasn't caused any problems (yet).

Curious what your thoughts are on using OpenFEC or wirehair for your application. WH is a bit heavier but has less overhead I think?

@catid
Copy link
Owner

catid commented Jan 1, 2021

Since your library operates in a stream fashion, another interesting alternative might be this library: https://github.com/catid/CauchyCaterpillar

The advantage is that instead of waiting for a block of data to arrive to recover, you can recover almost immediately next time a recovery packet arrives.

@xqms
Copy link
Author

xqms commented Jan 2, 2021

Curious what your thoughts are on using OpenFEC or wirehair for your application. WH is a bit heavier but has less overhead I think?

I've been using OpenFEC before, but it's been giving me memory leaks and sometimes even segmentation faults. Some of them went away after importing some third-party patches (don't quite remember right now where I got them from). In any case, I had the impression that OpenFEC is not actively maintained anymore, so I looked for alternatives.
Wirehair was in comparison very easy to integrate and seems very robust.

So it's less theoretical or performance arguments but more technical and practical arguments that caused me to switch.

Since your library operates in a stream fashion, another interesting alternative might be this library: https://github.com/catid/CauchyCaterpillar
The advantage is that instead of waiting for a block of data to arrive to recover, you can recover almost immediately next time a recovery packet arrives.

Interesting, I'll take a look! It's true that nimbro_network operates on a stream of data, but right now each message in the stream (a ROS message) is given separately to WH for encoding - which I think is a reasonable solution. Small messages (<MTU) simply get duplicated, larger messages get the more efficient WH coding. But CauchyCaterpillar looks like it could be applied on the UDP packet level, which would probably be more elegant...
I'm only working on nimbro_network when our research group has need for it, which is not the case right now. But it's sure to come up again, and then I'll definitely look at it 👍

@catid
Copy link
Owner

catid commented Jan 3, 2021

Cool. Thanks for letting me know about OpenFEC. It might be interesting to build my own version of this staircase binary + GF(256) random code type library that's more robust. There's no advantage to using Reed Solomon codes AFAIK for the heavy part of the matrix, and in fact there is some other kind of matrix structure that seems to do better when concatenated with a binary code that seems to not be studied in the literature, which I incorporated into Wirehair a year or two ago after stumbling on it by accident. Could maybe make some improvements to the design?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants