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

Enabling multithreading #312

Closed
bjv-capra opened this issue May 5, 2021 · 14 comments
Closed

Enabling multithreading #312

bjv-capra opened this issue May 5, 2021 · 14 comments

Comments

@bjv-capra
Copy link

  • Hardware description: STM32F7
  • RTOS: FreeRTOS
  • Installation type: Static lib
  • Version or commit hash: Foxy

I'm not following on #180 as not to stack deep down that thread something irrelevant.

I have some questions regarding what do the different flags really mean when enabling multithread support.

Even though I want to run multiple nodes, as it is right now, I don't need them to communicate between them. Thus, can I then not enable UCLIENT_PROFILE_SHARED_MEMORY? Furthermore, as I won't have nodes communicating between them, and if the SHARED_MEMORY is disabled, do I need to set anything for these two flags?

My use case would be the following

1 Node simply checking status (pinging the agent and other basic stuff, plus providing the setup)
1 Node with a subscriber
1 Node with a publisher and a subscriber
1 Node with a subscriber

This could also increase later, but as it's right now, the nodes are simply used as peripherals, they don't have any logic themselves.

I think I know what needs to be modified and taken into account on the colcon.meta, and how to adjust the rmw to support the entities + nodes, etc.

Thanks

@pablogs9
Copy link
Member

pablogs9 commented May 5, 2021

Hello @bjv-capra,

If you check the middleware CMake, right now in develop UCLIENT_PROFILE_SHARED_MEMORY is being enabled by default when UCLIENT_PROFILE_MULTITHREAD is enabled. But the shared memory engine will only act when a local publisher and subscriber match.

Regarding the other two flags, you don't need to set them because they have default values. But as long as you are not going to use the shared memory feature, you can set then to zero to save static memory.

@bjv-capra
Copy link
Author

I think I've got it, however, I've run into an issue with the tracetools not liking my -fno-rtti flag. Even though I have set

"tracetools": {
            "cmake-args": [
                "-DTRACETOOLS_DISABLED=ON",
                "-DTRACETOOLS_STATUS_CHECKING_TOOL=OFF",
            ]
        }

I've never had this issue before...

tracetools/include/tracetools/utils.hpp:65:34: error: cannot use 'typeid' with '-fno-rtti'
   65 |   return _demangle_symbol(typeid(l).name());

Do you prefer if I open a new issue regarding this? I haven't been able to verify that it builds correctly yet.

@pablogs9
Copy link
Member

pablogs9 commented May 5, 2021

Yes please, open a new issue for that explaining the steps to reproduce the issue (your CMake toolchain, steps, compiler version...)

@bjv-capra
Copy link
Author

I'm reopening the issue as I'm not being able to run under the intended use. I've enabled multithreading, however, I still see the following limitations.
Each node has to be manually initialized one at a time.
I can't ping the agent and have nodes running at the same time.
As mentioned in the first comment,
My use case would be the following

1 Node simply checking status (pinging the agent and other basic stuff, plus providing the setup)
1 Node with a subscriber
1 Node with a publisher and a subscriber
1 Node with a subscriber
Where the first node checking the status HAS and is launched first, it also provides a handle for the other nodes to initialize the node and executor (dependant on the support). The rest of the nodes could initialize simultaneously.

@bjv-capra bjv-capra reopened this May 5, 2021
@bjv-capra
Copy link
Author

If I make the nodes initialize one by one (after the support), and if I remove the periodic ping to the agent, then it seems to work fine, however, this worked the same without multithreading enabled.

@pablogs9
Copy link
Member

pablogs9 commented May 5, 2021

If you initialize the nodes in a thread and then launch the multiple threads for spinning the executors, it works as expected?

@pablogs9
Copy link
Member

pablogs9 commented May 5, 2021

@bjv-capra
Copy link
Author

I'd say, most likely, I don't have a way to check, however, I'm making sure the nodes are initialized sequentially (which would happen if I do it in a single thread).
However, I still can't ping the agent while the other nodes are being used. On the other hand, could it be a problem because of trying to ping the agent AND trying to initialize a node at the same time? I'd assume it shouldn't be a problem, but maybe it is?

@bjv-capra
Copy link
Author

Okay, that was what I was thinking, maybe I should just put on hold the ping feature until it's thread-safe

@bjv-capra
Copy link
Author

Thanks for the quick response! I'll get back soon with some results :). I'll close the issue in the meantime

@pablogs9
Copy link
Member

pablogs9 commented May 5, 2021

Cool, we are looking forward to hearing your feedback. If you have any other problems please tell us and we will try to improve de functionality asap.

@Acuadros95
Copy link
Contributor

Update: We are working on this issue on the following PR. After it is merged the ping functionality should be thread safe, along with all the transport communications.

@bjv-capra
Copy link
Author

Awesome, thanks for the quick response

@pablogs9
Copy link
Member

pablogs9 commented May 7, 2021

eProsima/Micro-XRCE-DDS-Client#234 merged, please rebuild your micro-ROS libraries and tell us if everything works as expected

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

3 participants