Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

From: Mohamad Chaarawi (mschaara_at_[hidden])
Date: 2007-07-25 22:45:32


In the current code, almost all #ifs are due to the fact that we don't
want to add the extra memory by the sparse parameters that are added to
the group structure.
The additional parameters are 5 pointers and 3 integers.
If nobody objects, i would actually keep those extra parameters, even if
sparse groups are disabled (in the default case on configure), because it
would reduce the number of #ifs in the code to only 2 (as i recall that i
had it before) ..

Thank you,

Mohamad

On Wed, July 25, 2007 4:23 pm, Brian Barrett wrote:
> 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
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>

-- 
Mohamad Chaarawi
Instructional Assistant		  http://www.cs.uh.edu/~mschaara
Department of Computer Science	  University of Houston
4800 Calhoun, PGH Room 526        Houston, TX 77204, USA