named-data-iot / ndn-riot

NDN stack module for RIOT-OS
8 stars 8 forks source link

face-table: remove malloc/free #1

Closed cgundogan closed 8 years ago

cgundogan commented 8 years ago

Remove calls to malloc() and free() in the face-table by using a static table with constant size. NDN_FACE_ENTRIES_NUMOF defines the face-table size and is currently set to 10 entries by default. NDN_FACE_ENTRIES_NUMOF can also be overwritten at compile-time with CFLAGS+="-DNDN_FACE_ENTRIES_NUMOF=X".

wentaoshang commented 8 years ago

Sorry for the late response (just got back from summer vacation). The changes look good to me. I'll have @cawka take a look and the merge it.

@cgundogan Do you think we should also apply similar changes to other NDN data structures (PIT, CS, FIB)? It seems necessary if we want to provide better real-time support but I still have concerns about wasted memory for the static allocations.

cgundogan commented 8 years ago

@wentaoshang adjusting the other data structures to use static instead of dynamic memory allocation is sensible. I did some initial work to adjust the FIB and I can show you a diff in the following days.

Regarding your concerns, it's true that we will need to fine tune the memory parameter for each use case if we go with statically allocated memory, but this way we get much more determinism and, as you mentioned, real-time support.

So I am in favour of going all the way and replace all calls to malloc et. al.

wentaoshang commented 8 years ago

There is one special (and important) case in ndn-riot design that I haven't found a good solution yet: we want to use "shared pointer" mechanism to manage the ownership of the NDN packets so that the app thread and NDN thread can process the shared packet without additional copying (e.g., the app thread creates data; the ndn thread stores it in the cache). We considered using gnrc_pktsnip_t (which already supports shared pointer semantics) to store the NDN packets. But this raises concern that the NDN thread may hold the packet for too long (e.g., in the cache) and eventually cause gnrc_pktbuf to run out of static memory. So currently for each new packet from the network we have to make an extra copy into our own shared pointer struct and release the pktsnip memory immediately after that.

The requirement for sharing packet memory across threads and holding it for an indefinite amount of time seems to go against the use of static memory, unless we pre-allocate large buffer just for that purpose. (Sorry if I didn't explain the issue clear enough...)