Trax Image Formats: Shared Memory

[cybertk]

Regarding to #29. I'm considering implement a shared memory solution based on POSIX <sys/shm.h>. The changes of code should be

1. Add a new type TRAX_IMAGE_SHM with protocol text shm

2. In libtrax, add a new shared memory container trax_shm_region, it's struct should be

   struct trax_shm_region {
       char* shm_id;
       void* ptr;
   }

3. trax_shm_region will attach to Image.data for TRAX_IMAGE_SHM type.

4. For Python binding, will add a new class ShmImage in image.py

The communication flow should be

1. TraxClient requests a shared_memory_region identified by shm_id through shm_open() and mmap()
2. TraxClient fills the image into shared_memory_region, and send trax text shm://trax-NNNN;width;height;format
3. TraxServer receives trax text and parses the shm_id
4. TraxServer attaches to the shared_memory_region identified by shm_id

Other notes

1. Shared memory should works in POSIX system only, Linux and macOS should be supported.

[lukacu]

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.

[cybertk]

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

1. Treat trax.shmem like trax.version, it should be manipulated by Trax internally also.

   TraxServer will set trax.shmem=yes and advertise it with @TRAX::hello if running on POSIX OS

2. Use trax_property to exchange the identity of shared memory region(shm_id)

   TraxClient generates shm_id and properties.set("shm_id", NNNN) , then TraxServer get it with properties.get("shm_id").

[lukacu]

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 shmem+data:// and shmem+image://to identify such cases and then parse the shared memory location and metadata (format, size). A few of internal functions in trax.c would have to be augmented with more arguments but I think that this approach follows the original idea very closely.

[cybertk]

Encoding shared memory related info into URI is good.

I have a bit confusing with existing Trax URI format, i.e. image://X;Y;Z;. I don't think this whole string is a valid URI according to the definition in RFC 3986, it seems only the front part image://X is following URI syntax.

So for encoding the shared memory URI, there should be two formats

1. shmem+data://X?shm_id=SHMID;Y;Z;
2. shmem+data://X;Y;Z;SHMID;

[lukacu]

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.

[cybertk]

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 shm in front part of scheme, we should add it's value(SHMID) in front part of body for better consistency.

So the final URI comes to be shm+image://SHMID;X;Y;Z;, changes against existing URI are

1. Add prefix shm+ in URI scheme
2. Add prefix SHMID; in URI body

[lukacu]

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.

[lukacu]

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.

[cybertk]

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 trax_image_create_memory/buffer() will still allocate real memory, and then copies entire data to the shared memory region when sending to Trax Server. This method involved an additional memory copy step, I don't think it's necessary and good.

To avoid the additional memory copy, we can support Shared Memory at more deeper level, that means trax_image_create_memory/buffer() will allocate shared memory region by default, and send the base64 encoded string to the Trax Server who does not support Shared Memory. This way will treat Shared Memory as a basic feature in the Trax, not the optional.

[lukacu]

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.

[lukacu]

Any progress?

[cybertk]

I implemented a different version and used it in our projects. You can find it at https://github.com/votchallenge/trax/compare/master...cybertk:support-shm-draft

For what we talked in this thread, I have not start the actual coding.

[lukacu]

Interesting, what are the main differences in relation to what we discussed here?

[lukacu]

Ping?

[cybertk]

It's just what I describe in the first comment

[lukacu]

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.