ETCLabs / RDMnet

Implementation of ANSI E1.33
https://etclabs.github.io/RDMnetDocs
Apache License 2.0
41 stars 12 forks source link

Initial API requirements #24

Closed samkearney closed 4 years ago

samkearney commented 4 years ago

/AzurePipelines run

azure-pipelines[bot] commented 4 years ago
Azure Pipelines successfully started running 1 pipeline(s).
samkearney commented 4 years ago

@nwagner

any implications to how the different memory styles change?

In addition to making the APIs nicer, it also makes memory management easier and improves cache locality. The library has two memory usage paradigms:

  1. malloc() is used (RDMNET_DYNAMIC_MEM=1) - messages are built as the TCP stream comes in off the wire, with realloc() called if necessary to add more items to an array, the equivalent of push_back() to a vector - the message is delivered to a callback once complete.
  2. Static memory is used (RDMNET_DYNAMIC_MEM=0), and messages are only parsed and delivered from a single thread (RDMNET_ALLOW_EXTERNALLY_MANAGED_SOCKETS=0) - static storage is defined large enough to hold X messages, which are delivered to a callback from a thread, then that storage is reused on the next iteration. Much simpler than the current method actually, and nicer for cache.

I'm a little uneasy about the change from having the compiler calculate size totals to having it in a comment.

Why?

nwagner commented 4 years ago

@samkearney, the uneasiness is a very minor one -- what if there was a math error in your comment? I'm fine with you keeping it as is.

samkearney commented 4 years ago

@nwagner I made that change because not only does it look cleaner at the site of the definition, it also looks much nicer when someone hovers over a use of the macro when they have some sort of Intellisense available. They instantly know the total without having to do the addition themselves.

I'm confident in the ability of unit tests and code review to catch any potential math errors.

nwagner commented 4 years ago

Your Intellisense point is well taken. Thanks!