-
Notifications
You must be signed in to change notification settings - Fork 404
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
Map segments by name instead of access permissions, and allow publishers write access to multiple #2128
Comments
@elBoberido Because of the additional FFI benefits I'd be inclined to pursue this solution more over #2092 if it is deemed feasible. I'm also cautiously optimistic that it is a better fit with the current architecture, since segments already have the notion of |
A few year ago when we created the design for mixed criticality we also though about a more fine grained approach but dismissed it. I don't remember anymore why but in the meantime requirements also changed and maybe we need to rethink how to approach the problem. With the iceoryx Rust implementation we have this more fine grained access and it should be possible to port some of the changes to the C++ implementation. Saying that, maybe we need to have a deep dive on the Rust design for the memory allocation. Could be done as special developer meetup. @elfenpiff what do you think? I agree that this solution would be better than just exposing the MemPoolInfo. Regarding the comment in the code. Since there is only one runtime and the publisher ctor does not allow to specify which segment to use, the only option was to restrict the writer segment to one per process. This is not a technical limitation though, just a design decision at that time. It wouldn't be a big problem to change it. We just need to adjust the API ... or even better, create an alternative API which also takes care of other things like having multiple runtimes in one process. I like the idea but we should check with elfenpiff what he has for the Rust implementation. |
I'd be thrilled to do this 🙂 FYI I've been working on a proof of concept. So far I've gotten to the point where I've successfully configured segments to be mapped by name instead of writer group, and I have a publisher example publishing onto three different segments. I now need to look into the subscriber side of things to figure out what implications this will have. In theory nothing needs to be done there, but I might also want to support configuring subscribers to only listen on certain segments. This probably would make it easier to design a solution to only pin the segments we want to on the subscriber side. |
I'm thinking something along the lines of an optional "filter" whereby a user can provide an optional list of shared memory segment names and the subscriber only maps those shared memory segments (assuming it also has read access, otherwise an error shall occur). The default would be the current behavior of listening on all segments where the subscriber has read access. |
What do you think of meetup on Tuesday 19th 5pm CET. This week we are kind of busy with the preparation for the initial Rust release. Everybody is welcome to join but I would rather keep the round smaller so I don't announce the meeting on gitter. People interested in the topic should notice the meeting from the comments in this issue. |
That time should work great for me 🙂 |
Was a great talk like always. A pity @elfenpiff couldn't make it. Looking forward to a small design document and patches :) |
Was a pleasure for me as well. Regarding the design document and patches -
|
Definitely. The smaller the PRs the easier it is to review them.
If it is something that might be a feature of its own but is a requirement to create this feature then it might make sense to have a new issue. Quite often we just have a list of task required to implement the feature in one issue and have multiple PRs. There can also be multiple PRs per task.
You can have multiple commits in a PR and it is also appreciated to not squash commits once the PR is opened to make it easier to track the changes. |
…ure and allow multiple write-access segments to be mapped
…ure and allow multiple write-access segments to be mapped
…ure and allow multiple write-access segments to be mapped
…ure and allow multiple write-access segments to be mapped
…ure and allow multiple write-access segments to be mapped
…ake the segment name into account
…ake the segment name into account
…ake the segment name into account
FYI I haven't forgotten about this. I've just been a bit bogged down with other matters lately. |
@gpalmer-latai no problem. It is also not inconvenient for me as I am having a lot of fun with the German bureaucracy because of the company setup :) |
Brief feature description
Shared memory segments are currently mapped strictly by access controls. It would be beneficial to map instead by name or some other key so that processes aren't limited to writing to a single segment and we could better split up communication channels across multiple segments.
Detailed information
Currently, it is possible to creating multiple shared memory segments with different access permissions, as described in the configuration guide.
However, not only do the segments allow one to specify access controls, but they create really strong couplings based on those access controls. The implications include:
The last point is of particular interest to me because I can consider a couple notable reasons why one would want to be able to create many shared memory segments and be able to access them from within the same process:
CUDA_PINNED_HOST
memory, and then each publisher / subscriber that needs access to such memory would identify that segment and pin the memory of only that segment.Possible Obstacles
I've found a curious comment in the code:
I have not dug around enough in the code to understand why only one memory manager can be supported per process and what it might take to support more. I'd appreciate more context on this statement if anyone knows more.
From my perhaps naive standpoint - I would expect that the proper restriction be that we only have one memory manager per publisher, but that within the same process we may have multiple publishers writing to possibly different segments. For example - one publisher publishing images with the help of some GPU acceleration taking advantage of the pinned host memory, and another publisher publishing some small metrics data that does not need pinned host memory, so it uses a different memory segment.
Possible Implementation
I am imagining extending the Roudi config so that it could conceivably look like
And then when creating a publisher one can optionally specify the name of a segment:
such that the name will be used directly to map the shared memory region rather than the access group of current process, as is currently done.
For backwards compatibility then, we could simply make the name default to user of the current process.
The text was updated successfully, but these errors were encountered: