Bug 1461556 - Don't use PodCopy/PodMove to implement typed-array element-to-element copying: bog-standard std::copy and std::copy_n are readily optimized to the same thing, and they don't have a non-obvious requirement that the type being copied be trivial. r=jandem
authorJeff Walden <jwalden@mit.edu>
Wed, 16 May 2018 19:00:54 -0700
changeset 418913 04a9320069d7
parent 418912 7a053ded7f80
child 418914 8d85342160ac
push id103419
push userjwalden@mit.edu
push dateFri, 18 May 2018 19:33:51 +0000
treeherdermozilla-inbound@7658d2d1e0d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1461556
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1461556 - Don't use PodCopy/PodMove to implement typed-array element-to-element copying: bog-standard std::copy and std::copy_n are readily optimized to the same thing, and they don't have a non-obvious requirement that the type being copied be trivial. r=jandem
js/src/vm/TypedArrayObject-inl.h
--- a/js/src/vm/TypedArrayObject-inl.h
+++ b/js/src/vm/TypedArrayObject-inl.h
@@ -9,17 +9,18 @@
 
 /* Utilities and common inline code for TypedArray */
 
 #include "vm/TypedArrayObject.h"
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Compiler.h"
 #include "mozilla/FloatingPoint.h"
-#include "mozilla/PodOperations.h"
+
+#include <algorithm>
 
 #include "jsnum.h"
 
 #include "builtin/Array.h"
 #include "gc/Zone.h"
 #include "jit/AtomicOperations.h"
 #include "js/Conversions.h"
 #include "js/Value.h"
@@ -211,22 +212,34 @@ class UnsharedOps
 
     template<typename T>
     static void memmove(SharedMem<T*> dest, SharedMem<T*> src, size_t size) {
         ::memmove(dest.unwrapUnshared(), src.unwrapUnshared(), size);
     }
 
     template<typename T>
     static void podCopy(SharedMem<T*> dest, SharedMem<T*> src, size_t nelem) {
-        mozilla::PodCopy(dest.unwrapUnshared(), src.unwrapUnshared(), nelem);
+        // std::copy_n better matches the argument values/types of this
+        // function, but as noted below it allows the input/output ranges to
+        // overlap.  std::copy does not, so use it so the compiler has extra
+        // ability to optimize.
+        const auto* first = src.unwrapUnshared();
+        const auto* last = first + nelem;
+        auto* result = dest.unwrapUnshared();
+        std::copy(first, last, result);
     }
 
     template<typename T>
-    static void podMove(SharedMem<T*> dest, SharedMem<T*> src, size_t nelem) {
-        mozilla::PodMove(dest.unwrapUnshared(), src.unwrapUnshared(), nelem);
+    static void podMove(SharedMem<T*> dest, SharedMem<T*> src, size_t n) {
+        // std::copy_n copies from |src| to |dest| starting from |src|, so
+        // input/output ranges *may* permissibly overlap, as this function
+        // allows.
+        const auto* start = src.unwrapUnshared();
+        auto* result = dest.unwrapUnshared();
+        std::copy_n(start, n, result);
     }
 
     static SharedMem<void*> extract(TypedArrayObject* obj) {
         return SharedMem<void*>::unshared(obj->viewDataUnshared());
     }
 };
 
 template<typename T, typename Ops>