On Jul 25, 2007, at 3:14 PM, Jeff Squyres wrote:

On Jul 25, 2007, at 5:07 PM, Brian Barrett wrote:

It just adds a lot of #if's throughout the code.  Other than that,
there's no reason to remove it.

I agree, lots of #ifs are bad.  But I guess I don't see the problem.
The only real important thing people were directly accessing in the
ompi_group_t is the array of proc pointers.  Indexing into them could
be done with a static inline function that just has slightly
different time complexity based on compile options.  Static inline
function that just does an index in the group proc pointer would have
almost no added overhead (none if the compiler doesn't suck).

Ya, that's what I proposed.  :-)

But I did also propose removing the extra #if's so that the sparse
group code would be available and we'd add an extra "if" in the
critical code path.

But we can do it this way instead:

Still use the MACRO to access proc_t's.  In the --disable-sparse-
groups scenario, have it map to comm.group.proc[i].  In the --enable-
sparse-groups scenario, have it like I listed in the original proposal:

     static inline ompi_proc_t lookup_group(ompi_group_t *group, int
index) {
         if (group_is_dense(group)) {
             return group->procs[index];
         } else {
             return sparse_group_lookup(group, index);
         }
     }

With:

a) groups are always dense if --enable and an MCA parameter turns off
sparse groups, or
b) there's an added check in the inline function for whether the MCA
parameter is on

I'm personally in favor of a) because it means only one conditional
in the critical path.

I don't really care about the sparse groups turned on case. I just want minimal #ifs in the global code and to not have an if() { ... } in the critical path when sparse groups are disabled :).

Brian

Reply via email to