Skip to content

Commit

Permalink
Avoid array allocation for moving to equal domain
Browse files Browse the repository at this point in the history
chpl__coerceMove(_array) already has the ability to omit an
array copy when the source and destination arrays refer to the
same domain.

However, when the domains have equal value but are separate instances,
chpl__coerceMove would allocate a separate array and then move elements
into it from the original array.

This leads to unnecessary memory usage in some cases; for example, a
recursive function that returns an array and so declares the return
type.

This commit adds a mechanism for array implementations to provide a way
to steal the data buffers from another array which can be used in this
scenario. In some ways this is similar to 'doiOptimizedSwap'. Note that
'doiOptimizedSwap' cannot be used here because we want to avoid
allocation for one of the arrays.

---
Signed-off-by: Michael Ferguson <[email protected]>
  • Loading branch information
mppf committed Sep 6, 2024
1 parent 3284b94 commit b743521
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 8 deletions.
28 changes: 23 additions & 5 deletions modules/internal/ChapelArray.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -3084,11 +3084,29 @@ module ChapelArray {

param moveElts = !typeMismatch;

// If the domains point to the same thing (and aren't just identical),
// then we can simply return the RHS array.
// TODO: if the domain types match, could steal data pointers
if moveElts && dom._instance == rhs.domain._instance {
return rhs;
if moveElts {
// If the domains point to the same thing (and aren't just identical),
// then we can simply return the RHS array.
if dom._instance == rhs.domain._instance {
return rhs;
}
// If the domains have the same value but aren't pointing to the same
// thing, we can ask the array implementation to steal the buffer
if Reflection.canResolveMethod(dom._value,
"doiBuildArrayMoving",
rhs._value) {
if dom == rhs.domain &&
dom.distribution == rhs.domain.distribution &&
!rhs._unowned {
// ask the array implementation to steal the buffer
pragma "no copy" // avoid error about recursion for initCopy
pragma "unsafe" // when eltType is non-nilable
var lhs = dom.buildArrayMoving(rhs._value);
// destroy the husk that we moved from
_do_destroy_arr(rhs._unowned, rhs._instance, deinitElts=false);
return lhs;
}
}
}

pragma "no copy" // avoid error about recursion for initCopy
Expand Down
26 changes: 26 additions & 0 deletions modules/internal/ChapelDomain.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -1730,6 +1730,7 @@ module ChapelDomain {
}

// assumes that data is already initialized
// this function is used in Fortran interop
pragma "no copy return"
@chpldoc.nodoc
proc buildArrayWith(type eltType, data:_ddata(eltType), allocSize:int) {
Expand All @@ -1748,6 +1749,31 @@ module ChapelDomain {
return _newArray(x);
}

// assumes that the caller has checked:
// * that 'from' and the receiver domain match & have compatible types
// * that the distributions match as well
// * that the 'from array is not unowned
// * that the domain/array implementation supports doiBuildArrayMoving
pragma "no copy return"
@chpldoc.nodoc
proc buildArrayMoving(from) {
var x = _value.doiBuildArrayMoving(from);
pragma "dont disable remote value forwarding"
proc help() {
_value.add_arr(x);
}
help();

chpl_incRefCountsForDomainsInArrayEltTypes(x, x.eltType);

return _newArray(x);

// note: 'from' will be deinited here, normally leading to
// deleting the array instance. The array implementation needs
// to have set anything stolen to 'nil' in doiBuildArrayMoving
// and then to take no action on it when deleting.
}

/*
An instance of this type is a context manager that can be used in
manage statements to resize arrays of non-default-initializable
Expand Down
24 changes: 21 additions & 3 deletions modules/internal/DefaultRectangular.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -717,20 +717,33 @@ module DefaultRectangular {
}

proc dsiBuildArrayWith(type eltType, data:_ddata(eltType), allocSize:int) {

var allocRange:range(idxType) = (ranges(0).lowBound)..#allocSize;
return new unmanaged DefaultRectangularArr(eltType=eltType,
rank=rank,
idxType=idxType,
strides=strides,
dom=_to_unmanaged(this),
// consider the elements already inited
initElts=false,
// but the array should deinit them
deinitElts=true,
dom=_to_unmanaged(this),
data=data);
}

proc doiBuildArrayMoving(from: DefaultRectangularArr(?)) {
var movedData = from.data;
from.data = nil; // so that it won't be freed when 'from' is deleted
return new unmanaged DefaultRectangularArr(
eltType=from.eltType,
rank=rank,
idxType=idxType,
strides=strides,
// consider the elements already inited
initElts=false,
// but the array should deinit them
deinitElts=true,
dom=_to_unmanaged(this),
data=movedData);
}

proc dsiLocalSlice(ranges) {
halt("all dsiLocalSlice calls on DefaultRectangulars should be handled in ChapelArray.chpl");
Expand Down Expand Up @@ -1137,6 +1150,11 @@ module DefaultRectangular {
}

override proc dsiDestroyArr(deinitElts:bool) {
// give up early if the array's data has already been stolen
if data == nil {
return;
}

if debugDefaultDist {
chpl_debug_writeln("*** DR calling dealloc ", eltType:string);
}
Expand Down

0 comments on commit b743521

Please sign in to comment.