swb: replace SetArray with CopyArray

Address CR feedback.

Author: Jianchun Xu

lib/Common/DataStructures/BaseDictionary.h

@@ -93,14 +93,14 @@ namespace JsUtil
         Pointer(int, TAllocator) buckets;
         Pointer(EntryType, TAllocator) entries;
         PointerNoBarrier(AllocatorType) alloc;
-        int size;
-        uint bucketCount;
-        int count;
-        int freeList;
-        int freeCount;
+        Field(int) size;
+        Field(uint) bucketCount;
+        Field(int) count;
+        Field(int) freeList;
+        Field(int) freeCount;
 
 #if PROFILE_DICTIONARY
-        DictionaryStats *stats;
+        PointerNoBarrier(DictionaryStats) stats;
 #endif
         enum InsertOperations
         {
@@ -179,11 +179,8 @@ namespace JsUtil
             freeList = other.freeList;
             freeCount = other.freeCount;
 
-            size_t copySize = bucketCount * sizeof(buckets[0]);
-            js_memcpy_s(buckets, copySize, other.buckets, copySize);
-
-            copySize = size * sizeof(entries[0]);
-            js_memcpy_s(entries, copySize, other.entries, copySize);
+            CopyArray<TAllocator>(buckets, bucketCount, other.buckets, bucketCount);
+            CopyArray<TAllocator, EntryType, ValueType>(entries, size, other.entries, size);
 
 #if PROFILE_DICTIONARY
             stats = DictionaryStats::Create(typeid(this).name(), size);
@@ -698,11 +695,8 @@ namespace JsUtil
             freeList = other->freeList;
             freeCount = other->freeCount;
 
-            size_t copySize = bucketCount * sizeof(buckets[0]);
-            js_memcpy_s(buckets, copySize, other->buckets, copySize);
-
-            copySize = size * sizeof(entries[0]);
-            js_memcpy_s(entries, copySize, other->entries, copySize);
+            CopyArray<TAllocator>(buckets, bucketCount, other->buckets, bucketCount);
+            CopyArray<TAllocator, EntryType, ValueType>(entries, size, other->entries, size);
 
 #if PROFILE_DICTIONARY
             stats = DictionaryStats::Create(typeid(this).name(), size);
@@ -866,8 +860,8 @@ namespace JsUtil
             Allocate(&newBuckets, &newEntries, initBucketCount, initSize);
 
             // Allocation can throw - assign only after allocation has succeeded.
-            SetArray<TAllocator>(this->buckets, newBuckets, initBucketCount);
-            SetArray<TAllocator, EntryType, ValueType>(this->entries, newEntries, initSize);
+            this->buckets = newBuckets;
+            this->entries = newEntries;
             this->bucketCount = initBucketCount;
             this->size = initSize;
             Assert(this->freeCount == 0);
@@ -1007,7 +1001,7 @@ namespace JsUtil
             {
                 // no need to rehash
                 newEntries = AllocateEntries(newSize);
-                js_memcpy_s(newEntries, sizeof(EntryType) * newSize, entries, sizeof(EntryType) * count);
+                CopyArray<TAllocator, EntryType, ValueType>(newEntries, newSize, entries, count);
 
                 DeleteEntries(entries, size);
 
@@ -1017,7 +1011,7 @@ namespace JsUtil
             }
 
             Allocate(&newBuckets, &newEntries, newBucketCount, newSize);
-            js_memcpy_s(newEntries, sizeof(EntryType) * newSize, entries, sizeof(EntryType) * count);
+            CopyArray<TAllocator, EntryType, ValueType>(newEntries, newSize, entries, count);
 
             // When TAllocator is of type Recycler, it is possible that the Allocate above causes a collection, which
             // in turn can cause entries in the dictionary to be removed - i.e. the dictionary contains weak references
@@ -1042,8 +1036,8 @@ namespace JsUtil
             if (stats)
                 stats->Resize(newSize, /*emptyBuckets=*/ newSize - size);
 #endif
-            SetArray<TAllocator>(this->buckets, newBuckets, newBucketCount);
-            SetArray<TAllocator, EntryType, ValueType>(this->entries, newEntries, newSize);
+            this->buckets = newBuckets;
+            this->entries = newEntries;
             bucketCount = newBucketCount;
             size = newSize;
         }
@@ -1565,7 +1559,7 @@ namespace JsUtil
     class SynchronizedDictionary: protected BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry>
     {
     private:
-        SyncObject* syncObj;
+        PointerNoBarrier(SyncObject) syncObj;
 
         typedef BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry> Base;
     public:
@@ -1817,4 +1811,3 @@ namespace JsUtil
         PREVENT_COPY(SynchronizedDictionary);
     };
 }
-

lib/Common/DataStructures/DefaultContainerLockPolicy.h

@@ -14,7 +14,7 @@ namespace Js
         {
         public:
             template <class SyncObject>
-            NoLock(SyncObject*)
+            NoLock(const SyncObject&)
             {
                 // No lock, do nothing.
             }

lib/Common/Memory/RecyclerPointers.h

@@ -108,34 +108,33 @@ void WriteBarrier(T * address, size_t count)
     return _ArrayWriteBarrier<Policy>::WriteBarrier(address, count);
 }
 
-// Set an array field to an allocated array.
-//
-// Will trigger write barrier on the array content When the field is of WriteBarrierPtr
-// type, and Allocator / element type determines write barrier required.
+// Copy array content. Triggers write barrier on the dst array content if if
+// Allocator and element type determines write barrier is needed.
 //
 template <class Allocator, class T, class PolicyType = T>
-void SetArray(T*& dst, T* src, size_t srcCount)
+void CopyArray(T* dst, size_t dstCount, const T* src, size_t srcCount)
 {
-    dst = src;
+    js_memcpy_s(dst, sizeof(T) * dstCount, src, sizeof(T) * srcCount);
+    WriteBarrier<T, Allocator, PolicyType>(dst, dstCount);
 }
 template <class Allocator, class T, class PolicyType = T>
-void SetArray(NoWriteBarrierPtr<T>& dst, T* src, size_t srcCount)
+void CopyArray(const NoWriteBarrierPtr<T>& dst, size_t dstCount, const NoWriteBarrierPtr<T>& src, size_t srcCount)
 {
-    dst = src;
+    return CopyArray<Allocator, T, PolicyType>((T*)dst, dstCount, (const T*)src, srcCount);
 }
 template <class Allocator, class T, class PolicyType = T>
-void SetArray(WriteBarrierPtr<T>& dst, T* src, size_t srcCount)
+void CopyArray(const WriteBarrierPtr<T>& dst, size_t dstCount, const WriteBarrierPtr<T>& src, size_t srcCount)
 {
-    dst = src;
-    WriteBarrier<T, Allocator, PolicyType>(dst, srcCount);
+    return CopyArray<Allocator, T, PolicyType>((T*)dst, dstCount, (const T*)src, srcCount);
 }
 
+
 template <typename T>
 class NoWriteBarrierField
 {
 public:
     NoWriteBarrierField() {}
-    NoWriteBarrierField(T const& value) : value(value) {}
+    explicit NoWriteBarrierField(T const& value) : value(value) {}
 
     // Getters
     operator T const&() const { return value; }
@@ -160,7 +159,7 @@ class NoWriteBarrierPtr
 {
 public:
     NoWriteBarrierPtr() {}
-    NoWriteBarrierPtr(T * value) : value(value) {}
+    explicit NoWriteBarrierPtr(T * value) : value(value) {}
 
     // Getters
     T * operator->() const { return this->value; }
@@ -209,7 +208,7 @@ class WriteBarrierPtr
 {
 public:
     WriteBarrierPtr() {}
-    WriteBarrierPtr(T * ptr)
+    explicit WriteBarrierPtr(T * ptr)
     {
         // WriteBarrier
         NoWriteBarrierSet(ptr);