Bug 1763606 - Part 6: Copy shared memory for RadixSort. r=tcampbell

Similar to part 5, concurrent write accesses shouldn't affect the sort algorithm.

Depends on D143289

Differential Revision: https://phabricator.services.mozilla.com/D143290

js/src/tests/non262/TypedArray/sort_modifications_concurrent_radixsort.js

@@ -0,0 +1,116 @@
+// |reftest| skip-if(!xulRuntime.shell)
+
+if (helperThreadCount() === 0) {
+  if (typeof reportCompare === "function")
+    reportCompare(true, true);
+  quit();
+}
+
+// TypedArray constructors which can use radix sort.
+const TAConstructors = [
+  Int16Array,
+  Uint16Array,
+  Int32Array,
+  Uint32Array,
+  Float32Array,
+];
+
+// Use a large enough size to ensure concurrent accesses can be detected.
+const size = 0x4000;
+
+function ToAtomicTA(TA) {
+  switch (TA) {
+  case Int16Array:
+  case Int32Array:
+  case Uint16Array:
+  case Uint32Array:
+    return TA;
+  case Float32Array:
+    return Uint32Array;
+  }
+  throw new Error("Invalid typed array kind");
+}
+
+function ascending(a, b) {
+  return a < b ? -1 : a > b ? 1 : 0;
+}
+
+function descending(a, b) {
+  return -ascending(a, b);
+}
+
+for (let TA of TAConstructors) {
+  let sorted = new TA(size);
+
+  // Fill with |1..size| and then sort to account for wrap-arounds.
+  for (let i = 0; i < size; ++i) {
+    sorted[i] = i + 1;
+  }
+  sorted.sort();
+
+  let extra = Math.max(TA.BYTES_PER_ELEMENT, Int32Array.BYTES_PER_ELEMENT);
+  let buffer = new SharedArrayBuffer(size * TA.BYTES_PER_ELEMENT + extra);
+  let controller = new Int32Array(buffer, 0, 1);
+
+  // Create a copy in descending order.
+  let ta = new TA(buffer, extra, size);
+  ta.set(sorted)
+  ta.sort(descending);
+
+  // Worker code expects that the last element changes when sorted.
+  assertEq(ta[size - 1] === sorted[size - 1], false);
+
+  setSharedObject(buffer);
+
+  evalInWorker(`
+    const ToAtomicTA = ${ToAtomicTA};
+    const TA = ${TA.name};
+    const AtomicTA = ToAtomicTA(TA);
+
+    let size = ${size};
+    let extra = ${extra};
+
+    let buffer = getSharedObject();
+    let controller = new Int32Array(buffer, 0, 1);
+    let ta = new AtomicTA(buffer, extra, size);
+
+    let value = Atomics.load(ta, size - 1);
+
+    // Coordinate with main thread.
+    while (Atomics.notify(controller, 0, 1) < 1) ;
+
+    // Wait until modification of the last element.
+    //
+    // Sorting writes in ascending indexed ordered, so when the last element
+    // was modified, we know that the sort operation has finished.
+    while (Atomics.load(ta, size - 1) === value) ;
+
+    // Set all elements to zero.
+    ta.fill(0);
+
+    // Sleep for 50 ms.
+    const amount = 0.05;
+
+    // Coordinate with main thread.
+    while (Atomics.notify(controller, 0, 1) < 1) {
+      sleep(amount);
+    }
+  `);
+
+  // Wait until worker is set-up.
+  assertEq(Atomics.wait(controller, 0, 0), "ok");
+
+  // Sort the array in ascending order.
+  ta.sort();
+
+  // Wait until worker has finished.
+  assertEq(Atomics.wait(controller, 0, 0), "ok");
+
+  // All elements have been set to zero.
+  for (let i = 0; i < size; ++i) {
+    assertEq(ta[i], 0, `${TA.name} at index ${i} for size ${size}`);
+  }
+}
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);

js/src/vm/TypedArrayObject.cpp

@@ -2818,6 +2818,8 @@ template <typename T, typename U, typename Ops>
 static void SortByColumn(SharedMem<U*> data, size_t length, SharedMem<U*> aux,
                          uint8_t col) {
   static_assert(std::is_unsigned_v<U>, "SortByColumn sorts on unsigned values");
+  static_assert(std::is_same_v<Ops, UnsharedOps>,
+                "SortByColumn only works on unshared data");
 
   // |counts| is used to compute the starting index position for each key.
   // Letting counts[0] always be 0, simplifies the transform step below.
@@ -2856,14 +2858,8 @@ static void SortByColumn(SharedMem<U*> data, size_t length, SharedMem<U*> aux,
     U val = Ops::load(data + i);
     uint8_t b = ByteAtCol(val);
     size_t j = counts[b]++;
-    if constexpr (std::is_same_v<Ops, SharedOps>) {
-      // Watch out for concurrent writes on shared memory. Invoke the "sort
-      // order is implementation-defined" rule from the spec and leave the rest
-      // unsorted.
-      if (j >= length) {
-        return;
-      }
-    }
+    MOZ_ASSERT(j < length,
+               "index is in bounds when |data| can't be modified concurrently");
     UnsharedOps::store(aux + j, val);
   }
 
@@ -2907,8 +2903,30 @@ static bool TypedArrayRadixSort(JSContext* cx, TypedArrayObject* typedArray) {
   SharedMem<UnsignedT*> data =
       typedArray->dataPointerEither().cast<UnsignedT*>();
 
+  // Always create a copy when sorting shared memory backed typed arrays to
+  // ensure concurrent write accesses don't lead to computing bad indices.
+  SharedMem<UnsignedT*> unshared;
+  SharedMem<UnsignedT*> shared;
+  UniquePtr<UnsignedT[], JS::FreePolicy> ptrUnshared;
+  if constexpr (std::is_same_v<Ops, SharedOps>) {
+    ptrUnshared = cx->make_pod_array<UnsignedT>(length);
+    if (!ptrUnshared) {
+      return false;
+    }
+    unshared = SharedMem<UnsignedT*>::unshared(ptrUnshared.get());
+    shared = data;
+
+    Ops::podCopy(unshared, shared, length);
+
+    data = unshared;
+  }
+
   for (uint8_t col = 0; col < sizeof(UnsignedT); col++) {
-    SortByColumn<T, UnsignedT, Ops>(data, length, aux, col);
+    SortByColumn<T, UnsignedT, UnsharedOps>(data, length, aux, col);
+  }
+
+  if constexpr (std::is_same_v<Ops, SharedOps>) {
+    Ops::podCopy(shared, unshared, length);
   }
 
   return true;