Skip to content

Commit adda190

Browse files
markallemarkalle
committed
[from mpich] romio flatten has flat->count consistency problems
Merging a romio commit that was accepted by mpich: > romio flatten has flat->count consistency problems > > ADIOI_Count_contiguous_blocks and ADIOI_Flatten are very similar functions, > but they diverge a lot around the edges and in degenerate cases. In Spectrum > MPI I spent some time making them consistent with each other but > found that to be a losing battle. > > So the approach I used here is to not have Count() be as definitive, > and rather let Flatten() have the last word on what the final flat->count > really is. Eg Flatten's *curr_index is the real count. > > The changes I made are > > 1. Fix a couple places in Flatten where *curr_index was updated out > of sync with what was actually being added to flat->indices[] and > flat->blocklens[]. That's one of the important book-keeping rules > Flatten should follow. There were a couple places (when counts[] arrays > have 0s) where that wasn't the case (see the "nonzeroth" var in this > commit). > > 2. The main change I made was to reset flat->count based on > Flatten's curr_index. This is because the divergence between the > two functions is too great to reliably fix. > > 3. A third change is just a safety valve, using a > flatlist_node_grow() function just in case the > Count function returns a smaller value than Flatten ends up > trying to write into the array, and related to this I > got rid of the various --(flat->count) lines, since that var now > represents the allocated size of the array, until right after > the Flatten function when it's reset to represent the data written > to the array like it used to be. > > I don't think we even need ADIOI_Count_contiguous_blocks() anymore, > but I didn't remove it. Signed-off-by: Mark Allen <[email protected]>
1 parent 3e317d7 commit adda190

File tree

1 file changed

+67
-18
lines changed
  • ompi/mca/io/romio321/romio/adio/common

1 file changed

+67
-18
lines changed

‎ompi/mca/io/romio321/romio/adio/common/flatten.c‎

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,33 @@ int ADIOI_Type_get_contents (MPI_Datatype datatype, int max_integers,
9898
return rc;
9999
}
100100

101+
/*
102+
* I don't really expect this to ever trigger, but without the below safety
103+
* valve, the design relies on the Count function coming out >= whatever
104+
* the Flatten function comes up with. There are enough differences between
105+
* the two that it's hard to be positive this will always be true. So every
106+
* time something's added to flat's arrays, let's make sure they're big enough
107+
* and re-alloc if not.
108+
*/
109+
static void flatlist_node_grow(ADIOI_Flatlist_node * flat, int idx)
110+
{
111+
if (idx >= flat->count) {
112+
ADIO_Offset *new_blocklens;
113+
ADIO_Offset *new_indices;
114+
int new_count = (flat->count * 1.25 + 4);
115+
new_blocklens = (ADIO_Offset *) ADIOI_Calloc(new_count * 2, sizeof(ADIO_Offset));
116+
new_indices = new_blocklens + new_count;
117+
if (flat->count) {
118+
memcpy(new_blocklens, flat->blocklens, flat->count * sizeof(ADIO_Offset));
119+
memcpy(new_indices, flat->indices, flat->count * sizeof(ADIO_Offset));
120+
ADIOI_Free(flat->blocklens);
121+
}
122+
flat->blocklens = new_blocklens;
123+
flat->indices = new_indices;
124+
flat->count = new_count;
125+
}
126+
}
127+
101128
void ADIOI_Optimize_flattened(ADIOI_Flatlist_node *flat_type);
102129
/* flatten datatype and add it to Flatlist */
103130
void ADIOI_Flatten_datatype(MPI_Datatype datatype)
@@ -168,6 +195,16 @@ void ADIOI_Flatten_datatype(MPI_Datatype datatype)
168195
DBG_FPRINTF(stderr,"ADIOI_Flatten_datatype:: ADIOI_Flatten\n");
169196
#endif
170197

198+
/*
199+
* Setting flat->count to curr_index, since curr_index is the most fundamentally
200+
* correct updated value that represents what's in the indices/blocklens arrays.
201+
* It would be nice if the counter function and the flatten function were in sync,
202+
* but the numerous cases that decrement flat->count in the flatten function show
203+
* that syncing them is a hack, and as long as the counter doesn't under-count
204+
* it's good enough.
205+
*/
206+
flat->count = curr_index;
207+
171208
ADIOI_Optimize_flattened(flat);
172209
#endif
173210
/* debug */
@@ -318,6 +355,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
318355
if (prev_index == *curr_index) {
319356
/* simplest case, made up of basic or contiguous types */
320357
j = *curr_index;
358+
flatlist_node_grow(flat, j);
321359
flat->indices[j] = st_offset;
322360
MPI_Type_size_x(types[0], &old_size);
323361
flat->blocklens[j] = top_count * old_size;
@@ -335,6 +373,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
335373
MPI_Type_get_extent(types[0], &lb, &old_extent);
336374
for (m=1; m<top_count; m++) {
337375
for (i=0; i<num; i++) {
376+
flatlist_node_grow(flat, j);
338377
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
339378
flat->blocklens[j] = flat->blocklens[j-num];
340379
#ifdef FLATTEN_DEBUG
@@ -366,10 +405,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
366405
avoid >2G integer arithmetic problems */
367406
ADIO_Offset blocklength = ints[1], stride = ints[2];
368407
j = *curr_index;
408+
flatlist_node_grow(flat, j);
369409
flat->indices[j] = st_offset;
370410
MPI_Type_size_x(types[0], &old_size);
371411
flat->blocklens[j] = blocklength * old_size;
372412
for (i=j+1; i<j+top_count; i++) {
413+
flatlist_node_grow(flat, i);
373414
flat->indices[i] = flat->indices[i-1] + stride * old_size;
374415
flat->blocklens[i] = flat->blocklens[j];
375416
}
@@ -389,6 +430,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
389430
MPI_Type_get_extent(types[0], &lb, &old_extent);
390431
for (m=1; m<blocklength; m++) {
391432
for (i=0; i<num; i++) {
433+
flatlist_node_grow(flat, j);
392434
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
393435
flat->blocklens[j] = flat->blocklens[j-num];
394436
j++;
@@ -400,6 +442,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
400442
num = *curr_index - prev_index;
401443
for (i=1; i<top_count; i++) {
402444
for (m=0; m<num; m++) {
445+
flatlist_node_grow(flat, j);
403446
flat->indices[j] = flat->indices[j-num] + stride * ADIOI_AINT_CAST_TO_OFFSET old_extent;
404447
flat->blocklens[j] = flat->blocklens[j-num];
405448
j++;
@@ -429,10 +472,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
429472
avoid >2G integer arithmetic problems */
430473
ADIO_Offset blocklength = ints[1];
431474
j = *curr_index;
475+
flatlist_node_grow(flat, j);
432476
flat->indices[j] = st_offset;
433477
MPI_Type_size_x(types[0], &old_size);
434478
flat->blocklens[j] = blocklength * old_size;
435479
for (i=j+1; i<j+top_count; i++) {
480+
flatlist_node_grow(flat, i);
436481
flat->indices[i] = flat->indices[i-1] + adds[0];
437482
flat->blocklens[i] = flat->blocklens[j];
438483
}
@@ -452,6 +497,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
452497
MPI_Type_get_extent(types[0], &lb, &old_extent);
453498
for (m=1; m<blocklength; m++) {
454499
for (i=0; i<num; i++) {
500+
flatlist_node_grow(flat, j);
455501
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
456502
flat->blocklens[j] = flat->blocklens[j-num];
457503
j++;
@@ -463,6 +509,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
463509
num = *curr_index - prev_index;
464510
for (i=1; i<top_count; i++) {
465511
for (m=0; m<num; m++) {
512+
flatlist_node_grow(flat, j);
466513
flat->indices[j] = flat->indices[j-num] + adds[0];
467514
flat->blocklens[j] = flat->blocklens[j-num];
468515
j++;
@@ -500,16 +547,15 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
500547
avoid >2G integer arithmetic problems */
501548
ADIO_Offset blocklength = ints[1+i-j], stride = ints[top_count+1+i-j];
502549
if (blocklength > 0) {
550+
flatlist_node_grow(flat, nonzeroth);
503551
flat->indices[nonzeroth] =
504552
st_offset + stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
505553
flat->blocklens[nonzeroth] =
506554
blocklength* ADIOI_AINT_CAST_TO_OFFSET old_extent;
507555
nonzeroth++;
508-
} else {
509-
flat->count--; /* don't count/consider any zero-length blocklens */
510556
}
511557
}
512-
*curr_index = i;
558+
*curr_index = nonzeroth;
513559
}
514560
else {
515561
/* indexed type made up of noncontiguous derived types */
@@ -523,14 +569,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
523569
for (m=1; m<ints[1]; m++) {
524570
for (i=0, nonzeroth = j; i<num; i++) {
525571
if (flat->blocklens[j-num] > 0) {
572+
flatlist_node_grow(flat, nonzeroth);
526573
flat->indices[nonzeroth] =
527574
flat->indices[nonzeroth-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
528575
flat->blocklens[nonzeroth] =
529576
flat->blocklens[nonzeroth-num];
530577
j++;
531578
nonzeroth++;
532-
} else {
533-
flat->count --;
534579
}
535580
}
536581
}
@@ -545,26 +590,24 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
545590
avoid >2G integer arithmetic problems */
546591
ADIO_Offset stride = ints[top_count+1+i]-ints[top_count+i];
547592
if (flat->blocklens[j-num] > 0 ) {
593+
flatlist_node_grow(flat, nonzeroth);
548594
flat->indices[nonzeroth] =
549595
flat->indices[j-num] + stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
550596
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
551597
j++;
552598
nonzeroth++;
553-
} else {
554-
flat->count--;
555599
}
556600
}
557601
*curr_index = j;
558602
for (m=1; m<ints[1+i]; m++) {
559603
for (k=0, nonzeroth=j; k<basic_num; k++) {
560604
if (flat->blocklens[j-basic_num] > 0) {
605+
flatlist_node_grow(flat, nonzeroth);
561606
flat->indices[nonzeroth] =
562607
flat->indices[j-basic_num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
563608
flat->blocklens[nonzeroth] = flat->blocklens[j-basic_num];
564609
j++;
565610
nonzeroth++;
566-
} else {
567-
flat->count --;
568611
}
569612
}
570613
}
@@ -611,9 +654,11 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
611654
avoid >2G integer arithmetic problems */
612655
ADIO_Offset blocklength = ints[1];
613656
if (is_hindexed_block) {
657+
flatlist_node_grow(flat, i);
614658
flat->indices[i] = st_offset + adds[i-j];
615659
} else {
616660
ADIO_Offset stride = ints[1+1+i-j];
661+
flatlist_node_grow(flat, i);
617662
flat->indices[i] = st_offset +
618663
stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
619664
}
@@ -636,6 +681,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
636681
* extent of a type */
637682
MPI_Type_get_extent(types[0], &lb, &old_extent);
638683
}
684+
flatlist_node_grow(flat, j);
639685
flat->indices[j] = flat->indices[j-num] +
640686
ADIOI_AINT_CAST_TO_OFFSET old_extent;
641687
flat->blocklens[j] = flat->blocklens[j-num];
@@ -649,12 +695,14 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
649695
for (i=1; i<top_count; i++) {
650696
for (m=0; m<num; m++) {
651697
if (is_hindexed_block) {
698+
flatlist_node_grow(flat, j);
652699
flat->indices[j] = flat->indices[j-num] +
653700
adds[i] - adds[i-1];
654701
} else {
655702
/* By using ADIO_Offset we preserve +/- sign and
656703
avoid >2G integer arithmetic problems */
657704
ADIO_Offset stride = ints[2+i]-ints[1+i];
705+
flatlist_node_grow(flat, j);
658706
flat->indices[j] = flat->indices[j-num] +
659707
stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
660708
}
@@ -691,14 +739,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
691739
/* By using ADIO_Offset we preserve +/- sign and
692740
avoid >2G integer arithmetic problems */
693741
ADIO_Offset blocklength = ints[1+i-j];
742+
flatlist_node_grow(flat, nonzeroth);
694743
flat->indices[nonzeroth] = st_offset + adds[i-j];
695744
flat->blocklens[nonzeroth] = blocklength*old_size;
696745
nonzeroth++;
697-
} else {
698-
flat->count--;
699746
}
700747
}
701-
*curr_index = i;
748+
*curr_index = nonzeroth;
702749
}
703750
else {
704751
/* indexed type made up of noncontiguous derived types */
@@ -713,13 +760,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
713760
for (m=1; m<ints[1]; m++) {
714761
for (i=0, nonzeroth=j; i<num; i++) {
715762
if (flat->blocklens[j-num] > 0) {
763+
flatlist_node_grow(flat, nonzeroth);
716764
flat->indices[nonzeroth] =
717765
flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
718766
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
719767
j++;
720768
nonzeroth++;
721-
} else {
722-
flat->count--;
723769
}
724770
}
725771
}
@@ -731,19 +777,19 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
731777
prev_index = *curr_index;
732778
for (m=0, nonzeroth=j; m<basic_num; m++) {
733779
if (flat->blocklens[j-num] > 0) {
780+
flatlist_node_grow(flat, nonzeroth);
734781
flat->indices[nonzeroth] =
735782
flat->indices[j-num] + adds[i] - adds[i-1];
736783
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
737784
j++;
738785
nonzeroth++;
739-
} else {
740-
flat->count--;
741786
}
742787
}
743788
*curr_index = j;
744789
for (m=1; m<ints[1+i]; m++) {
745790
for (k=0,nonzeroth=j; k<basic_num; k++) {
746791
if (flat->blocklens[j-basic_num] >0) {
792+
flatlist_node_grow(flat, nonzeroth);
747793
flat->indices[nonzeroth] =
748794
flat->indices[j-basic_num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
749795
flat->blocklens[nonzeroth] = flat->blocklens[j-basic_num];
@@ -779,6 +825,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
779825
if (ints[1+n] > 0) {
780826
ADIO_Offset blocklength = ints[1+n];
781827
j = *curr_index;
828+
flatlist_node_grow(flat, j);
782829
flat->indices[j] = st_offset + adds[n];
783830
MPI_Type_size_x(types[n], &old_size);
784831
flat->blocklens[j] = blocklength * old_size;
@@ -798,6 +845,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
798845
MPI_Type_get_extent(types[n], &lb, &old_extent);
799846
for (m=1; m<ints[1+n]; m++) {
800847
for (i=0; i<num; i++) {
848+
flatlist_node_grow(flat, j);
801849
flat->indices[j] =
802850
flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
803851
flat->blocklens[j] = flat->blocklens[j-num];
@@ -827,6 +875,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
827875
* bound based on the inner type, but the lower bound based on the
828876
* upper type. check both lb and ub to prevent mixing updates */
829877
if (flat->lb_idx == -1 && flat->ub_idx == -1) {
878+
flatlist_node_grow(flat, j);
830879
flat->indices[j] = st_offset + adds[0];
831880
/* this zero-length blocklens[] element, unlike eleswhere in the
832881
* flattening code, is correct and is used to indicate a lower bound
@@ -843,7 +892,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
843892
} else {
844893
/* skipped over this chunk because something else higher-up in the
845894
* type construction set this for us already */
846-
flat->count--;
847895
st_offset -= adds[0];
848896
}
849897

@@ -859,6 +907,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
859907
else {
860908
/* current type is basic or contiguous */
861909
j = *curr_index;
910+
flatlist_node_grow(flat, j);
862911
flat->indices[j] = st_offset;
863912
MPI_Type_size_x(types[0], &old_size);
864913
flat->blocklens[j] = old_size;
@@ -874,6 +923,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
874923
/* see note above about mixing updates for why we check lb and ub */
875924
if ((flat->lb_idx == -1 && flat->ub_idx == -1) || lb_updated) {
876925
j = *curr_index;
926+
flatlist_node_grow(flat, j);
877927
flat->indices[j] = st_offset + adds[0] + adds[1];
878928
/* again, zero-element ok: an upper-bound marker explicitly set by the
879929
* constructor of this resized type */
@@ -882,7 +932,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
882932
} else {
883933
/* skipped over this chunk because something else higher-up in the
884934
* type construction set this for us already */
885-
flat->count--;
886935
(*curr_index)--;
887936
}
888937

0 commit comments

Comments
 (0)