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
> de...@open-mpi.org
> 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
