-
Notifications
You must be signed in to change notification settings - Fork 46
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
Trax Image Formats: Shared Memory #31
Comments
I like the idea of offloading the data-intensive part of messages to shared memory, but I am not sure if this is the best approach, at least not by introducing another image type for this. In terms of format the proposed approach is no different than the in-memory type, the only difference is the channel of transportation. Therefore I am more in favor of doing this internally and not exposing it to the user. We can report the support for shared memory in handshake using something like trax.shmem argument. Then the client would be aware of this possibility and decide if it can use it (e.g. in case of sending files or raw data). The philosophy of the protocol is to offload decisions like this to the client and keep the tracker as simple as possible so that researchers are not bothered with communication details too much. |
It's better if we make fewer changes on Trax protocol and user side APIs to enable shared memory. To support shared memory internally, as you suggest, we can let Trax handle it automatically through negotiation. There is another difference if we support shared memory internally, it can be used by both TRAX_IMAGE_BUFFER and TRAX_IMAGE_MEMORY. Here are some my thoughts on the implementation
|
I agree with the first point. I would also add an option to explicitly disable support via environmental variable (if you want to make some tests). About the second point I do not see why something like this would be necessary. The option that you have mentioned in the first post is completely fine, but it has to be done internally when decoding the image. I would recommend URI prefixes |
Encoding shared memory related info into URI is good. I have a bit confusing with existing Trax URI format, i.e. So for encoding the shared memory URI, there should be two formats
|
Hm, I must say that I was not very diligent when it comes to RFC definition of URI, perhaps we can change this one day. My focus was on easy parsing as every detail that would make sense in RFC is very painful to implement in pure C. |
That makes sense, beside that I suggest use keyword shm instead of shmem in scheme part. Shorter is better, and POSIX also use shm representing SHared Memory. Another point is if we add So the final URI comes to be
|
Yes, the proposed suggestions look ok. I think that we have all the major issues settled now, the only implementation suggestion that I would make is to standardize the rule that a shared memory is only guaranteed to be valid until the next TRAX message. This means that you have to copy the memory if you want to keep for a longer period of time (which will be done in many cases anyway because you cannot wrap the memory directly). It will also be useful because memory ids can be reused. If you will have any other questions regarding internal structure of the library do ask. |
By the way, there is a possibility that something similar could also be done on Windows using memory mapped files, just something to keep in mind. |
There is another issue while implementing the above solution, I find it's hard to support Shared Memory as a optional feature. In above solution, Shared Memory support is enabled through Trax negotiation, so it's should be a communication level feature(Pass content through 1/ shared memory ID vs 2/ base64 encoded string). If it's a communication level feature, that means To avoid the additional memory copy, we can support Shared Memory at more deeper level, that means |
I see what you mean. I still think that we should do it implicitly, even if the memory is copied twice (this would still be much faster than encoding it to a stream). The problem with having shared memory allocated directly is that it can be also freed directly, if not managed by some global internal state. This may result in all kinds of problem (like releasing a shared memory at the wrong moment and probably crashing the tracker). The compromise solution would be that all the create functions would require trax handle as an input and that this would ensure some consistency and also allocation of appropriate memory. This means that the API would be significantly modified which is again something that I would like to avoid, but it would be less problematic than explicit memory handling. |
Any progress? |
I implemented a different version and used it in our projects. You can find it at master...cybertk:support-shm-draft For what we talked in this thread, I have not start the actual coding. |
Interesting, what are the main differences in relation to what we discussed here? |
Ping? |
It's just what I describe in the first comment |
Ah, ok. Thanks. I will have a look at your fork if there is anything that I can use when I will have time to get to it. Keep me posted if you will make any progress. |
Regarding to #29. I'm considering implement a shared memory solution based on POSIX <sys/shm.h>. The changes of code should be
Add a new type
TRAX_IMAGE_SHM
with protocol textshm
In
libtrax
, add a new shared memory containertrax_shm_region
, it's struct should betrax_shm_region
will attach toImage.data
forTRAX_IMAGE_SHM
type.For Python binding, will add a new class ShmImage in image.py
The communication flow should be
shared_memory_region
identified by shm_id throughshm_open()
andmmap()
shared_memory_region
, and send trax textshm://trax-NNNN;width;height;format
shared_memory_region
identified by shm_idOther notes
The text was updated successfully, but these errors were encountered: