[MERGE #1797 @jianchun] swb: annotate BaseDictionary (partial)

Jianchun Xu · Jianchun Xu · commit e22ec433a2b9 · 2016-10-24T18:05:10.000-07:00
Merge pull request #1797 from jianchun:wballoc Expand `Pointer` macro to choose `WriteBarrierPtr/NoWriteBarrierPtr` based on optional `Allocator` type using `WriteBarrierPtrTraits`. (E.g. pointer fields but using with Arena, choose NoWriteBarrierPtr.) Add `WriteBarrier` function to trigger write barrier on array content when needed. Added `CopyArray` function to copy array data. Triggers write barrier on the array content when needed. By default only pointer types, WriteBarrierPtr wrapper, or explicit _write_barrier_policy trigger write barrier (when using Recycler). Users can also use template instantiation to alter custom allocator/type write barrier policy. This change does not handle individual `Item` write barrier yet.
diff --git a/lib/Common/DataStructures/BaseDictionary.h b/lib/Common/DataStructures/BaseDictionary.h
@@ -90,17 +90,17 @@ namespace JsUtil
         friend class Js::RemoteDictionary<BaseDictionary>;
         template <typename ValueOrKey> struct ComparerType { typedef Comparer<ValueOrKey> Type; }; // Used by diagnostics to access Comparer type
 
-        int* buckets;
-        EntryType* entries;
-        AllocatorType* alloc;
-        int size;
-        uint bucketCount;
-        int count;
-        int freeList;
-        int freeCount;
+        Pointer(int, TAllocator) buckets;
+        Pointer(EntryType, TAllocator) entries;
+        PointerNoBarrier(AllocatorType) alloc;
+        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);
@@ -860,9 +854,14 @@ namespace JsUtil
             int initSize = max(capacity, 4);
             uint initBucketCount = SizePolicy::GetBucketSize(initSize);
             AssertMsg(initBucketCount > 0, "Size returned by policy should be greater than 0");
-            Allocate(&buckets, &entries, initBucketCount, initSize);
 
-            // Allocation can throw - assign the size only after allocation has succeeded.
+            int* newBuckets = nullptr;
+            EntryType* newEntries = nullptr;
+            Allocate(&newBuckets, &newEntries, initBucketCount, initSize);
+
+            // Allocation can throw - assign only after allocation has succeeded.
+            this->buckets = newBuckets;
+            this->entries = newEntries;
             this->bucketCount = initBucketCount;
             this->size = initSize;
             Assert(this->freeCount == 0);
@@ -1002,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);
 
@@ -1012,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
@@ -1037,10 +1036,10 @@ namespace JsUtil
             if (stats)
                 stats->Resize(newSize, /*emptyBuckets=*/ newSize - size);
 #endif
-            buckets = newBuckets;
+            this->buckets = newBuckets;
+            this->entries = newEntries;
             bucketCount = newBucketCount;
             size = newSize;
-            entries = newEntries;
         }
 
         __ecount(bucketCount) int *AllocateBuckets(DECLSPEC_GUARD_OVERFLOW const uint bucketCount)
@@ -1560,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:
@@ -1812,4 +1811,3 @@ namespace JsUtil
         PREVENT_COPY(SynchronizedDictionary);
     };
 }
-
diff --git a/lib/Common/DataStructures/DefaultContainerLockPolicy.h b/lib/Common/DataStructures/DefaultContainerLockPolicy.h
@@ -14,7 +14,7 @@ namespace Js
         {
         public:
             template <class SyncObject>
-            NoLock(SyncObject*)
+            NoLock(const SyncObject&)
             {
                 // No lock, do nothing.
             }
diff --git a/lib/Common/Exceptions/Throw.cpp b/lib/Common/Exceptions/Throw.cpp
@@ -37,6 +37,7 @@ namespace Memory {
 using namespace Memory;
 #include "Memory/Allocator.h"
 #include "Memory/HeapAllocator.h"
+#include "Memory/RecyclerPointers.h"
 
 // Data structure
 #include "DataStructures/Comparer.h"
diff --git a/lib/Common/Memory/RecyclerPointers.h b/lib/Common/Memory/RecyclerPointers.h
@@ -6,12 +6,135 @@
 
 namespace Memory
 {
+class Recycler;
+class RecyclerNonLeafAllocator;
+
+// Dummy tag classes to mark yes/no write barrier policy
+//
+struct _write_barrier_policy {};
+struct _no_write_barrier_policy {};
+
+// Type write barrier policy
+//
+// By default following potentially contains GC pointers and use write barrier policy:
+//      pointer, WriteBarrierPtr, _write_barrier_policy
+//
+template <class T>
+struct TypeWriteBarrierPolicy { typedef _no_write_barrier_policy Policy; };
+template <class T>
+struct TypeWriteBarrierPolicy<T*> { typedef _write_barrier_policy Policy; };
+template <class T>
+struct TypeWriteBarrierPolicy<WriteBarrierPtr<T>> { typedef _write_barrier_policy Policy; };
+template <>
+struct TypeWriteBarrierPolicy<_write_barrier_policy> { typedef _write_barrier_policy Policy; };
+
+// AllocatorType write barrier policy
+//
+// Recycler allocator type => _write_barrier_policy
+// Note that Recycler allocator type consists of multiple allocators:
+//      Recycler, RecyclerNonLeafAllocator, RecyclerLeafAllocator
+//
+template <class AllocatorType>
+struct _AllocatorTypeWriteBarrierPolicy { typedef _no_write_barrier_policy Policy; };
+template <>
+struct _AllocatorTypeWriteBarrierPolicy<Recycler> { typedef _write_barrier_policy Policy; };
+
+template <class Policy1, class Policy2>
+struct _AndWriteBarrierPolicy { typedef _no_write_barrier_policy Policy; };
+template <>
+struct _AndWriteBarrierPolicy<_write_barrier_policy, _write_barrier_policy>
+{
+    typedef _write_barrier_policy Policy;
+};
+
+// Combine Allocator + Data => write barrier policy
+// Specialize RecyclerNonLeafAllocator
+//
+template <class Allocator, class T>
+struct AllocatorWriteBarrierPolicy
+{
+    typedef typename AllocatorInfo<Allocator, void>::AllocatorType AllocatorType;
+    typedef typename _AndWriteBarrierPolicy<
+        typename _AllocatorTypeWriteBarrierPolicy<AllocatorType>::Policy,
+        typename TypeWriteBarrierPolicy<T>::Policy>::Policy Policy;
+};
+template <class T>
+struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, T> { typedef _write_barrier_policy Policy; };
+template <>
+struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, int> { typedef _no_write_barrier_policy Policy; };
+
+// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Policy
+//
+template <class T, class Policy>
+struct _WriteBarrierPtrPolicy { typedef NoWriteBarrierPtr<T> Ptr; };
+template <class T>
+struct _WriteBarrierPtrPolicy<T, _write_barrier_policy> { typedef WriteBarrierPtr<T> Ptr; };
+
+// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Allocator and T* type
+//
+template <class T,
+          class Allocator = Recycler,
+          class Policy = typename AllocatorWriteBarrierPolicy<Allocator, T*>::Policy>
+struct WriteBarrierPtrTraits { typedef typename _WriteBarrierPtrPolicy<T, Policy>::Ptr Ptr; };
+
+// ArrayWriteBarrier behavior
+//
+template <class Policy>
+struct _ArrayWriteBarrier
+{
+    template <class T>
+    static void WriteBarrier(T * address, size_t count) {}
+};
+
+#ifdef RECYCLER_WRITE_BARRIER
+template <>
+struct _ArrayWriteBarrier<_write_barrier_policy>
+{
+    template <class T>
+    static void WriteBarrier(T * address, size_t count)
+    {
+        RecyclerWriteBarrierManager::WriteBarrier(address, sizeof(T) * count);
+    }
+};
+#endif
+
+// Trigger write barrier on changing array content if Allocator and element type
+// determines write barrier is needed. Ignore otherwise.
+//
+template <class T, class Allocator = Recycler, class PolicyType = T>
+void WriteBarrier(T * address, size_t count)
+{
+    typedef typename AllocatorWriteBarrierPolicy<Allocator, PolicyType>::Policy Policy;
+    return _ArrayWriteBarrier<Policy>::WriteBarrier(address, count);
+}
+
+// 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 CopyArray(T* dst, size_t dstCount, const T* src, size_t srcCount)
+{
+    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 CopyArray(NoWriteBarrierPtr<T>& dst, size_t dstCount, const NoWriteBarrierPtr<T>& src, size_t srcCount)
+{
+    return CopyArray<Allocator, T, PolicyType>((T*)dst, dstCount, (const T*)src, srcCount);
+}
+template <class Allocator, class T, class PolicyType = T>
+void CopyArray(WriteBarrierPtr<T>& dst, size_t dstCount, const WriteBarrierPtr<T>& src, size_t 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; }
@@ -136,23 +259,17 @@ class WriteBarrierPtr
     static void MoveArray(WriteBarrierPtr * dst, WriteBarrierPtr * src, size_t count)
     {
         memmove((void *)dst, src, sizeof(WriteBarrierPtr) * count);
-#ifdef RECYCLER_WRITE_BARRIER
-        RecyclerWriteBarrierManager::WriteBarrier(dst, count);
-#endif
+        WriteBarrier(dst, count);
     }
     static void CopyArray(WriteBarrierPtr * dst, size_t dstCount, T const* src, size_t srcCount)
     {
         js_memcpy_s((void *)dst, sizeof(WriteBarrierPtr) * dstCount, src, sizeof(T *) * srcCount);
-#ifdef RECYCLER_WRITE_BARRIER
-        RecyclerWriteBarrierManager::WriteBarrier(dst, dstCount);
-#endif
+        WriteBarrier(dst, dstCount);
     }
     static void CopyArray(WriteBarrierPtr * dst, size_t dstCount, WriteBarrierPtr const* src, size_t srcCount)
     {
         js_memcpy_s((void *)dst, sizeof(WriteBarrierPtr) * dstCount, src, sizeof(WriteBarrierPtr) * srcCount);
-#ifdef RECYCLER_WRITE_BARRIER
-        RecyclerWriteBarrierManager::WriteBarrier(dst, dstCount);
-#endif
+        WriteBarrier(dst, dstCount);
     }
     static void ClearArray(WriteBarrierPtr * dst, size_t count)
     {
@@ -162,7 +279,7 @@ class WriteBarrierPtr
 private:
     T * ptr;
 };
-}
+}  // namespace Memory
 
 
 template<class T> inline
diff --git a/lib/Common/Memory/RecyclerWriteBarrierManager.cpp b/lib/Common/Memory/RecyclerWriteBarrierManager.cpp
@@ -57,7 +57,7 @@ X64WriteBarrierCardTableManager::OnThreadInit()
     ULONG_PTR stackEnd = 0;
     ::GetCurrentThreadStackLimits(&stackEnd, &stackBase);
 #endif
-    
+
     size_t numPages = (stackBase - stackEnd) / AutoSystemInfo::PageSize;
     // stackEnd is the lower boundary
     return OnSegmentAlloc((char*) stackEnd, numPages);
@@ -283,11 +283,11 @@ RecyclerWriteBarrierManager::WriteBarrier(void * address)
 
 
 void
-RecyclerWriteBarrierManager::WriteBarrier(void * address, size_t ptrCount)
+RecyclerWriteBarrierManager::WriteBarrier(void * address, size_t bytes)
 {
 #ifdef RECYCLER_WRITE_BARRIER_BYTE
     uintptr_t startIndex = GetCardTableIndex(address);
-    char * endAddress = (char *)Math::Align<INT_PTR>((INT_PTR)((char *)address + sizeof(void *) * ptrCount), s_WriteBarrierPageSize);
+    char * endAddress = (char *)Math::Align<INT_PTR>((INT_PTR)((char *)address + bytes), s_WriteBarrierPageSize);
     uintptr_t endIndex = GetCardTableIndex(endAddress);
     Assert(startIndex <= endIndex);
     memset(cardTable + startIndex, 1, endIndex - startIndex);
@@ -297,7 +297,7 @@ RecyclerWriteBarrierManager::WriteBarrier(void * address, size_t ptrCount)
     uint bitMask = 0xFFFFFFFF << bitShift;
     uint cardIndex = ((uint)address) / s_BytesPerCard);
 
-    char * endAddress = (char *)Math::Align((INT_PTR)((char *)address + sizeof(void *) * ptrCount), s_BytesPerCardBit);
+    char * endAddress = (char *)Math::Align((INT_PTR)((char *)address + bytes), s_BytesPerCardBit);
     char * alignedAddress = (char *)Math::Align((INT_PTR)address, s_WriteBarrierPageSize);
     if (alignedAddress > endAddress)
     {
diff --git a/lib/Common/Memory/RecyclerWriteBarrierManager.h b/lib/Common/Memory/RecyclerWriteBarrierManager.h
@@ -123,7 +123,7 @@ class RecyclerWriteBarrierManager
 {
 public:
     static void WriteBarrier(void * address);
-    static void WriteBarrier(void * address, size_t ptrCount);
+    static void WriteBarrier(void * address, size_t bytes);
 
     // For JIT
     static uintptr_t GetCardTableIndex(void * address);
diff --git a/lib/Common/Memory/WriteBarrierMacros.h b/lib/Common/Memory/WriteBarrierMacros.h
@@ -12,7 +12,7 @@
 // Usage:
 //   In general, don't need to use anything, just use the macros to annotate your fields
 //
-//   If you want to force your fields to always generate the right write-barrier types, 
+//   If you want to force your fields to always generate the right write-barrier types,
 //   at the start of the file, add the following:
 //     #define FORCE_USE_WRITE_BARRIER
 //     #include <Memory/WriteBarrierMacros.h>
@@ -27,34 +27,34 @@
 #define PushMacro(x) __pragma(push_macro( #x ))
 
 #define SAVE_WRITE_BARRIER_MACROS() \
-    PushMacro("PointerNoBarrier") \
     PushMacro("Field") \
+    PushMacro("PointerNoBarrier") \
     PushMacro("Pointer")
 
 #define RESTORE_WRITE_BARRIER_MACROS() \
-    PopMacro("PointerNoBarrier") \
     PopMacro("Field") \
+    PopMacro("PointerNoBarrier") \
     PopMacro("Pointer")
 
 #endif
 
 #ifdef FORCE_USE_WRITE_BARRIER
 SAVE_WRITE_BARRIER_MACROS()
-#undef PointerNoBarrier
 #undef Field
+#undef PointerNoBarrier
 #undef Pointer
 #endif
 
 // TODO: Turn off these annotations on Win32
 #if defined(__clang__) || defined(FORCE_USE_WRITE_BARRIER)
 // Various macros for defining field attributes
-#define PointerNoBarrier(type) NoWriteBarrierPtr<type>
 #define Field(type) NoWriteBarrierField<type>
-#define Pointer(type) WriteBarrierPtr<type>
+#define PointerNoBarrier(type) NoWriteBarrierPtr<type>
+#define Pointer(type, ...) typename WriteBarrierPtrTraits<type, ##__VA_ARGS__>::Ptr
 #else
-#define PointerNoBarrier(type) type*
 #define Field(type) type
-#define Pointer(type) type*
+#define PointerNoBarrier(type) type*
+#define Pointer(type, ...) type*
 #endif
 
 #undef FORCE_USE_WRITE_BARRIER
diff --git a/lib/Runtime/Base/AuxPtrs.h b/lib/Runtime/Base/AuxPtrs.h
diff --git a/lib/Runtime/Base/ScriptContext.cpp b/lib/Runtime/Base/ScriptContext.cpp

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ namespace Js`
`14`	`14`	`{`
`15`	`15`	`public:`
`16`	`16`	`template <class SyncObject>`
`17`		`- NoLock(SyncObject*)`
	`17`	`+ NoLock(const SyncObject&)`
`18`	`18`	`{`
`19`	`19`	`// No lock, do nothing.`
`20`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -123,7 +123,7 @@ class RecyclerWriteBarrierManager`
`123`	`123`	`{`
`124`	`124`	`public:`
`125`	`125`	`static void WriteBarrier(void * address);`
`126`		`- static void WriteBarrier(void * address, size_t ptrCount);`
	`126`	`+ static void WriteBarrier(void * address, size_t bytes);`
`127`	`127`
`128`	`128`	`// For JIT`
`129`	`129`	`static uintptr_t GetCardTableIndex(void * address);`