[Openframeworkwg] source code for proof of concept framework available

[Jason Gunthorpe]

On Thu, Nov 07, 2013 at 09:10:13PM +0000, Hefty, Sean wrote:
> I created a git tree and published the source code based on the slides that I've been covering in the OFWG meetings.  I added a few comments in various places and in the initial commit message.  The source itself should be viewed as one possible instantiation of the framework, so we can have more concrete discussions.  The framework provider code is provided as a proof of concept and is not intended as some sort of final solution.
> 
> A browsable source tree is at:
> 
> http://git.openfabrics.org/git?p=~shefty/libfabric.git;a=summary

I took a few mins of looking, just some random structural comments
- Public headers need to be cautious of what names they introduce into
  the global space. If the global name doesn't start with 'fi_' then
  some underscores are needed eg:
  51  #define container_of
  46 typedef uint16_t be16_t;
  81 typedef unsigned long __attribute__((aligned(4))) packed_ulong;
  94 #define UMAD_IOCTL_MAGIC        0x1b
  [and more starting with umad_, maybe fi_ib_umad]

- I always hated that sockets exposed the network byte order
  to apps, IMHO that should be avoided unless it is on a high speed
  path ..
- const and type correctness - I didn't check deeply enough
  but there were a few bare 'char *' in some signatures, might be
  right, or maybe should be 'const char *' or ' void *'. This needs to be
  right everywhere or it is a PITA to use from C++
- It is also worth understanding the proper use of restrict and
  no except.
- fe_errno.h ... either have sane library
  specific unique error constants and xlate the kernel syscall errnos
  to the correct context specific error code,
  or stick with POSIX errno codes everwhere..
  Did you decide on an consistent 'errors returned in int' or
  'errors as errno scheme'?
- fi_atomic and fl_arch can probably use gcc intrinsics - introducing the
  multi-threaded memory model in C++11 caused gcc to gain a full set
  of memory barrier and atomic builtins for C code.

The downside of having 'ops' pointers neatly organized into
functionally group'd structures is now you pay a double dereference
cost at every call site, verbs had only a single dereference.

Also, the complete loss of static type safety by having 'fid_t' be the
only argument type seems like a big negative to me..

Cheers,
-- 
Jason Gunthorpe <jgunthorpe at obsidianresearch.com>        (780)4406067x832
Chief Technology Officer, Obsidian Research Corp         Edmonton, Canada