From 82069c3dff18cc9d46cd1b208c8b294633a5646e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Fri, 6 May 2022 07:31:26 +0000 Subject: [PATCH] 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 --- ...sort_modifications_concurrent_radixsort.js | 116 ++++++++++++++++++ js/src/vm/TypedArrayObject.cpp | 36 ++++-- 2 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 js/src/tests/non262/TypedArray/sort_modifications_concurrent_radixsort.js diff --git a/js/src/tests/non262/TypedArray/sort_modifications_concurrent_radixsort.js b/js/src/tests/non262/TypedArray/sort_modifications_concurrent_radixsort.js new file mode 100644 index 0000000000000..c6de0f1ecec7a --- /dev/null +++ b/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); diff --git a/js/src/vm/TypedArrayObject.cpp b/js/src/vm/TypedArrayObject.cpp index c958aeb13037e..2ac6a992b5b47 100644 --- a/js/src/vm/TypedArrayObject.cpp +++ b/js/src/vm/TypedArrayObject.cpp @@ -2818,6 +2818,8 @@ template static void SortByColumn(SharedMem data, size_t length, SharedMem aux, uint8_t col) { static_assert(std::is_unsigned_v, "SortByColumn sorts on unsigned values"); + static_assert(std::is_same_v, + "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 data, size_t length, SharedMem aux, U val = Ops::load(data + i); uint8_t b = ByteAtCol(val); size_t j = counts[b]++; - if constexpr (std::is_same_v) { - // 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 data = typedArray->dataPointerEither().cast(); + // Always create a copy when sorting shared memory backed typed arrays to + // ensure concurrent write accesses don't lead to computing bad indices. + SharedMem unshared; + SharedMem shared; + UniquePtr ptrUnshared; + if constexpr (std::is_same_v) { + ptrUnshared = cx->make_pod_array(length); + if (!ptrUnshared) { + return false; + } + unshared = SharedMem::unshared(ptrUnshared.get()); + shared = data; + + Ops::podCopy(unshared, shared, length); + + data = unshared; + } + for (uint8_t col = 0; col < sizeof(UnsignedT); col++) { - SortByColumn(data, length, aux, col); + SortByColumn(data, length, aux, col); + } + + if constexpr (std::is_same_v) { + Ops::podCopy(shared, unshared, length); } return true;