Avoid array allocation for moving to equal domain (#25896)

Resolves #25741

`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.

Future Work:
* add an implementation of `doiBuildArrayMoving` for more array types
(most notably, Block, Cyclic, and Stencil arrays)
* figure out how to get this optimization working for arrays-of-arrays.
Currently, they are disabled, because something is going wrong with
correctly setting up the domain for the nested arrays.

Reviewed by @benharsh - thanks!

- [x] primers pass with valgrind
- [x] full comm=none testing
- [x] full comm=gasnet oversubscribed testing

modules/internal/ChapelArray.chpl

@@ -3084,11 +3084,31 @@ 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 &&
+           eltType == rhs.eltType && // note: eltType might have runtime part
+           !isArrayType(eltType) && // arrays-of-arrays not working yet
+           !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

modules/internal/ChapelDomain.chpl

@@ -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) {
@@ -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

modules/internal/DefaultRectangular.chpl

@@ -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");
@@ -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);
       }

test/arrays/ferguson/array-initialization-patterns/post-alloc-dists.prediff

@@ -1,6 +1,6 @@
 #!/bin/bash
 # show only lines containing "alloc int"
-# show only lines after "alloc test begins"
-# show only lines before "alloc test begins"
+# show only lines after "alloc int test begins"
+# show only lines before "alloc int test ends"
 cat $2 | grep "alloc int" | grep -A 1000 "alloc int test begins" | grep -B 1000 "alloc int test ends" > $2.tmp
 mv $2.tmp $2

test/arrays/ferguson/array-initialization-patterns/post-alloc.good

@@ -32,9 +32,6 @@ alloc int test6
 alloc int test6t
 *** DR alloc int(64) 10
 *** DR calling postalloc int(64) 10
-*** DR alloc int(64) 10
-*** DR calling dealloc int(64)
-*** DR calling postalloc int(64) 10
 *** DR calling dealloc int(64)
 alloc int test7
 *** DR alloc int(64) 10

test/arrays/ferguson/array-initialization-patterns/post-alloc.prediff

@@ -1,6 +1,6 @@
 #!/bin/bash
 # show only lines containing 'alloc int'
-# show only lines after "alloc test begins"
-# show only lines before "alloc test begins"
+# show only lines after "alloc int test begins"
+# show only lines before "alloc int test ends"
 cat $2 | grep "alloc int" | grep -A 1000 "alloc int test begins" | grep -B 1000 "alloc int test ends" > $2.tmp
 mv $2.tmp $2

test/arrays/ferguson/array-initialization-patterns/return-array-declared-type.chpl

@@ -0,0 +1,16 @@
+// this test is intended as a reproducer for the
+// problem of too much memory allocated
+// shown in issue #25741
+
+config const n = 100_000;
+
+proc myfn(nn: int) : [0..<nn] int {
+  var ret: [0..<nn] int;
+  return ret;
+}
+
+proc main() {
+  writeln("alloc int test begins");
+  var x = myfn(n);
+  writeln("alloc int test ends");
+}

test/arrays/ferguson/array-initialization-patterns/return-array-declared-type.compopts

@@ -0,0 +1 @@
+--set debugDefaultDist=true

test/arrays/ferguson/array-initialization-patterns/return-array-declared-type.good

@@ -0,0 +1,4 @@
+alloc int test begins
+*** DR alloc int(64) 100000
+*** DR calling postalloc int(64) 100000
+alloc int test ends

test/arrays/ferguson/array-initialization-patterns/return-array-declared-type.prediff

@@ -0,0 +1,6 @@
+#!/bin/bash
+# show only lines containing 'alloc int'
+# show only lines after "alloc int test begins"
+# show only lines before "alloc int test ends"
+cat $2 | grep "alloc int" | grep -A 1000 "alloc int test begins" | grep -B 1000 "alloc int test ends" > $2.tmp
+mv $2.tmp $2