From 235d359a4aa1eba32ba9678c9b017a83942bf0ae Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Wed, 6 Nov 2019 13:35:07 -0500 Subject: [PATCH 1/4] added NativePerJobThreadIntPtrs to store multiple counters at the same time --- .../NativePerJobThreadIntPtrs.cs | 434 ++++++++++++++++++ .../NativePerJobThreadIntPtrs.cs.meta | 11 + 2 files changed, 445 insertions(+) create mode 100644 JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs create mode 100644 JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs.meta diff --git a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs new file mode 100644 index 0000000..4f04048 --- /dev/null +++ b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs @@ -0,0 +1,434 @@ +//----------------------------------------------------------------------- +// +// Copyright (c) Jackson Dunstan. See LICENSE.txt. +// +//----------------------------------------------------------------------- + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; +using Unity.Burst; +using Unity.Collections; +using Unity.Collections.LowLevel.Unsafe; +using Unity.Jobs.LowLevel.Unsafe; + +namespace JacksonDunstan.NativeCollections +{ + /// + /// Same as , but uses an array of integers + /// instead of single value. Takes advantage of the fact that we are + /// already using a full cache line to sum the single int. + /// + [NativeContainer] + [NativeContainerSupportsDeallocateOnJobCompletion] + [DebuggerTypeProxy(typeof(NativePerJobThreadIntPtrsDebugView))] + [DebuggerDisplay("Value = NativeArray")] + [StructLayout(LayoutKind.Sequential)] + public unsafe struct NativePerJobThreadIntPtrs : IDisposable + { + /// + /// An atomic write-only version of the object suitable for use in a + /// ParallelFor job + /// + [NativeContainer] + [NativeContainerIsAtomicWriteOnly] + public struct Parallel { + + /// + /// The number of int ptrs. + /// + public int Length; + + /// + /// Pointer to the value in native memory + /// + [NativeDisableUnsafePtrRestriction] + internal int* m_Buffer; + + /// + /// Thread index of the job using this object. This is set by Unity + /// and must have this exact name and type. + /// + [NativeSetThreadIndex] + internal int m_ThreadIndex; + +#if ENABLE_UNITY_COLLECTIONS_CHECKS + /// + /// A handle to information about what operations can be safely + /// performed on the object at any given time. + /// + internal AtomicSafetyHandle m_Safety; + + /// + /// Create a parallel version of the object + /// + /// + /// + /// Pointer to the value + /// + /// + /// + /// Atomic safety handle for the object + /// + internal Parallel(int length, int* value, AtomicSafetyHandle safety) + { + Length = length; + m_Buffer = value; + m_ThreadIndex = 0; + m_Safety = safety; + } +#else + /// + /// Create a parallel version of the object + /// + /// + /// + /// Pointer to the value + /// + internal Parallel(int* value) + { + m_Buffer = value; + m_ThreadIndex = 0; + } +#endif + + /// + /// Increment the stored value + /// + /// + /// + /// This object + /// + [WriteAccessRequired] + public void Increment(int index) + { + RequireWriteAccess(); + m_Buffer[IntsPerCacheLine * m_ThreadIndex + index]++; + } + + /// + /// Decrement the stored value + /// + /// + /// + /// This object + /// + [WriteAccessRequired] + public void Decrement(int index) + { + RequireWriteAccess(); + m_Buffer[IntsPerCacheLine * m_ThreadIndex + index]--; + } + + /// + /// Add to the stored value + /// + /// + /// + /// Value to add. Use negative values for subtraction. + /// + /// + /// + /// This object + /// + [WriteAccessRequired] + public void Add(int index, int value) + { + RequireWriteAccess(); + m_Buffer[IntsPerCacheLine * m_ThreadIndex + index] += value; + } + + /// + /// Throw an exception if the object isn't writable + /// + [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] + [BurstDiscard] + private void RequireWriteAccess() + { +#if ENABLE_UNITY_COLLECTIONS_CHECKS + AtomicSafetyHandle.CheckWriteAndThrow(m_Safety); +#endif + } + } + + /// + /// The number of int ptrs. + /// + public int Length; + + /// + /// Pointer to the value in native memory. Must be named exactly this + /// way to allow for [NativeContainerSupportsDeallocateOnJobCompletion] + /// + [NativeDisableUnsafePtrRestriction] + internal int* m_Buffer; + + /// + /// Allocator used to create the backing memory + /// + /// This field must be named this way to comply with + /// [NativeContainerSupportsDeallocateOnJobCompletion] + /// + internal Allocator m_AllocatorLabel; + + // These fields are all required when safety checks are enabled +#if ENABLE_UNITY_COLLECTIONS_CHECKS + /// + /// A handle to information about what operations can be safely + /// performed on the object at any given time. + /// + private AtomicSafetyHandle m_Safety; + + /// + /// A handle that can be used to tell if the object has been disposed + /// yet or not, which allows for error-checking double disposal. + /// + [NativeSetClassTypeToNullOnSchedule] + private DisposeSentinel m_DisposeSentinel; +#endif + + /// + /// The number of integers that fit into a CPU cache line + /// + private const int IntsPerCacheLine = JobsUtility.CacheLineSize / sizeof(int); + + /// + /// The number of integers that fit into a CPU cache line + /// + private readonly int NumCacheLines; + + /// + /// Allocate memory and set the initial value + /// + /// + /// + /// Allocator to allocate and deallocate with. Must be valid. + /// + /// + /// + /// Initial value of the allocated memory + /// + public NativePerJobThreadIntPtrs(int length, Allocator allocator) + { + // Require a valid allocator + if (allocator <= Allocator.None) + { + throw new ArgumentException( + "Allocator must be Temp, TempJob or Persistent", + "allocator"); + } + + // Need a multiple of the number of cache lines + Length = length; + NumCacheLines = (int)Math.Ceiling((double)length / IntsPerCacheLine); + + // Allocate the memory for the values + int bufferSize = JobsUtility.CacheLineSize * NumCacheLines * JobsUtility.MaxJobThreadCount; + m_Buffer = (int*)UnsafeUtility.Malloc( + bufferSize, + UnsafeUtility.AlignOf(), + allocator); + UnsafeUtility.MemClear(m_Buffer, bufferSize); + + // Store the allocator to use when deallocating + m_AllocatorLabel = allocator; + + // Create the dispose sentinel +#if ENABLE_UNITY_COLLECTIONS_CHECKS +#if UNITY_2018_3_OR_NEWER + DisposeSentinel.Create(out m_Safety, out m_DisposeSentinel, 0, allocator); +#else + DisposeSentinel.Create(out m_Safety, out m_DisposeSentinel, 0); +#endif +#endif + } + + /// + /// Get or set the contained value at the provided index + /// + /// This operation requires read access to the node for 'get' and write + /// access to the node for 'set'. + /// + /// + /// + /// The contained value at the provided index. + /// + public int this[int index] + { + get + { + RequireReadAccess(); + int value = 0; + for (int i = 0; i < JobsUtility.MaxJobThreadCount; ++i) + { + value += m_Buffer[IntsPerCacheLine * i + index]; + } + return value; + } + + [WriteAccessRequired] + set + { + RequireWriteAccess(); + *(m_Buffer + index) = value; + for (int i = 1; i < JobsUtility.MaxJobThreadCount; ++i) + { + m_Buffer[IntsPerCacheLine * i + index] = 0; + } + } + } + + /// + /// Get a version of this object suitable for use in a ParallelFor job + /// + /// + /// + /// A version of this object suitable for use in a ParallelFor job + /// + public Parallel GetParallel() + { +#if ENABLE_UNITY_COLLECTIONS_CHECKS + Parallel parallel = new Parallel(Length, m_Buffer, m_Safety); + AtomicSafetyHandle.UseSecondaryVersion(ref parallel.m_Safety); +#else + Parallel parallel = new Parallel(m_Buffer); +#endif + return parallel; + } + + /// + /// Check if the underlying unmanaged memory has been created and not + /// freed via a call to . + /// + /// This operation has no access requirements. + /// + /// This operation is O(1). + /// + /// + /// + /// Initially true when a non-default constructor is called but + /// initially false when the default constructor is used. After + /// is called, this becomes false. Note that + /// calling on one copy of this object doesn't + /// result in this becoming false for all copies if it was true before. + /// This property should not be used to check whether the object + /// is usable, only to check whether it was ever usable. + /// + public bool IsCreated + { + get + { + return m_Buffer != null; + } + } + + /// + /// Release the object's unmanaged memory. Do not use it after this. Do + /// not call on copies of the object either. + /// + /// This operation requires write access. + /// + /// This complexity of this operation is O(1) plus the allocator's + /// deallocation complexity. + /// + [WriteAccessRequired] + public void Dispose() + { + RequireWriteAccess(); + +// Make sure we're not double-disposing +#if ENABLE_UNITY_COLLECTIONS_CHECKS +#if UNITY_2018_3_OR_NEWER + DisposeSentinel.Dispose(ref m_Safety, ref m_DisposeSentinel); +#else + DisposeSentinel.Dispose(m_Safety, ref m_DisposeSentinel); +#endif +#endif + + UnsafeUtility.Free(m_Buffer, m_AllocatorLabel); + m_Buffer = null; + } + + /// + /// Set whether both read and write access should be allowed. This is + /// used for automated testing purposes only. + /// + /// + /// + /// If both read and write access should be allowed + /// + [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] + [BurstDiscard] + public void TestUseOnlySetAllowReadAndWriteAccess( + bool allowReadOrWriteAccess) + { +#if ENABLE_UNITY_COLLECTIONS_CHECKS + AtomicSafetyHandle.SetAllowReadOrWriteAccess( + m_Safety, + allowReadOrWriteAccess); +#endif + } + + /// + /// Throw an exception if the object isn't readable + /// + [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] + [BurstDiscard] + private void RequireReadAccess() + { +#if ENABLE_UNITY_COLLECTIONS_CHECKS + AtomicSafetyHandle.CheckReadAndThrow(m_Safety); +#endif + } + + /// + /// Throw an exception if the object isn't writable + /// + [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] + [BurstDiscard] + private void RequireWriteAccess() + { +#if ENABLE_UNITY_COLLECTIONS_CHECKS + AtomicSafetyHandle.CheckWriteAndThrow(m_Safety); +#endif + } + } + + /// + /// Provides a debugger view of . + /// + internal sealed class NativePerJobThreadIntPtrsDebugView + { + /// + /// The object to provide a debugger view for + /// + private NativePerJobThreadIntPtrs ptrs; + + /// + /// Create the debugger view + /// + /// + /// + /// The object to provide a debugger view for + /// + public NativePerJobThreadIntPtrsDebugView(NativePerJobThreadIntPtrs ptrs) + { + this.ptrs = ptrs; + } + + /// + /// Get the viewed object's value + /// + /// + /// + /// The viewed object's value + /// + public int this[int index] + { + get + { + return ptrs[index]; + } + } + } +} diff --git a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs.meta b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs.meta new file mode 100644 index 0000000..f68065a --- /dev/null +++ b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 87e73f6ca4f7441be9b9ea7916d081d5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From ff5e623947be6b3fb22c2fdfd5c1779b5838ecec Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 26 Nov 2019 15:04:51 -0500 Subject: [PATCH 2/4] formatting / documentation fixes as per code review; added index safety checks; fixed a bug where the data structure wont work with safety checks off --- .../NativePerJobThreadIntPtrs.cs | 117 ++++++++++-------- 1 file changed, 66 insertions(+), 51 deletions(-) diff --git a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs index 4f04048..3ab3a11 100644 --- a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs +++ b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs @@ -15,9 +15,14 @@ namespace JacksonDunstan.NativeCollections { /// - /// Same as , but uses an array of integers - /// instead of single value. Takes advantage of the fact that we are - /// already using a full cache line to sum the single int. + /// A pointer to an int array stored in native (i.e. unmanaged) memory. One + /// integer array is stored for each of the maximum number of job threads. + /// As of Unity 2018.2, this results in 8 KB of memory usage for every 16 + /// integers. The advantage over is that all + /// operations on are faster due to not being + /// atomic. The resulting array ints are collected with a loop. This is + /// therefore a good option when most usage is via + /// and memory usage is not a concern. /// [NativeContainer] [NativeContainerSupportsDeallocateOnJobCompletion] @@ -32,15 +37,16 @@ public unsafe struct NativePerJobThreadIntPtrs : IDisposable /// [NativeContainer] [NativeContainerIsAtomicWriteOnly] - public struct Parallel { + public struct Parallel + { /// - /// The number of int ptrs. + /// The number of integers stored in the array. /// public int Length; /// - /// Pointer to the value in native memory + /// Pointers to the integer values in native memory /// [NativeDisableUnsafePtrRestriction] internal int* m_Buffer; @@ -63,6 +69,10 @@ public struct Parallel { /// Create a parallel version of the object /// /// + /// + /// The number of integers stored in the array + /// + /// /// /// Pointer to the value /// @@ -82,11 +92,16 @@ internal Parallel(int length, int* value, AtomicSafetyHandle safety) /// Create a parallel version of the object /// /// + /// + /// The number of integers stored in the array + /// + /// /// /// Pointer to the value /// - internal Parallel(int* value) + internal Parallel(int length, int* value) { + Length = length; m_Buffer = value; m_ThreadIndex = 0; } @@ -95,28 +110,26 @@ internal Parallel(int* value) /// /// Increment the stored value /// - /// - /// - /// This object - /// [WriteAccessRequired] public void Increment(int index) { RequireWriteAccess(); +#if ENABLE_UNITY_COLLECTIONS_CHECKS + if (index < 0 || index >= Length) throw new ArgumentException("Index must be between 0 and Length - 1.", nameof(index)); +#endif m_Buffer[IntsPerCacheLine * m_ThreadIndex + index]++; } /// /// Decrement the stored value /// - /// - /// - /// This object - /// [WriteAccessRequired] public void Decrement(int index) { RequireWriteAccess(); +#if ENABLE_UNITY_COLLECTIONS_CHECKS + if (index < 0 || index >= Length) throw new ArgumentException("Index must be between 0 and Length - 1.", nameof(index)); +#endif m_Buffer[IntsPerCacheLine * m_ThreadIndex + index]--; } @@ -127,14 +140,13 @@ public void Decrement(int index) /// /// Value to add. Use negative values for subtraction. /// - /// - /// - /// This object - /// [WriteAccessRequired] public void Add(int index, int value) { RequireWriteAccess(); +#if ENABLE_UNITY_COLLECTIONS_CHECKS + if (index < 0 || index >= Length) throw new ArgumentException("Index must be between 0 and Length - 1.", nameof(index)); +#endif m_Buffer[IntsPerCacheLine * m_ThreadIndex + index] += value; } @@ -142,7 +154,6 @@ public void Add(int index, int value) /// Throw an exception if the object isn't writable /// [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] - [BurstDiscard] private void RequireWriteAccess() { #if ENABLE_UNITY_COLLECTIONS_CHECKS @@ -152,13 +163,14 @@ private void RequireWriteAccess() } /// - /// The number of int ptrs. + /// The number of integers stored in the array. /// - public int Length; + public readonly int Length; /// - /// Pointer to the value in native memory. Must be named exactly this - /// way to allow for [NativeContainerSupportsDeallocateOnJobCompletion] + /// Pointers to the integer values in native memory. Must be named + /// exactly this way to allow for + /// [NativeContainerSupportsDeallocateOnJobCompletion] /// [NativeDisableUnsafePtrRestriction] internal int* m_Buffer; @@ -201,17 +213,18 @@ private void RequireWriteAccess() /// Allocate memory and set the initial value /// /// - /// - /// Allocator to allocate and deallocate with. Must be valid. + /// + /// The number of logical integers to allocate. Initial value is 0 for + /// all array elements. /// /// - /// - /// Initial value of the allocated memory + /// + /// Allocator to allocate and deallocate with. Must be valid. /// public NativePerJobThreadIntPtrs(int length, Allocator allocator) { // Require a valid allocator - if (allocator <= Allocator.None) + if (UnsafeUtility.IsValidAllocator(allocator)) { throw new ArgumentException( "Allocator must be Temp, TempJob or Persistent", @@ -236,7 +249,7 @@ public NativePerJobThreadIntPtrs(int length, Allocator allocator) // Create the dispose sentinel #if ENABLE_UNITY_COLLECTIONS_CHECKS #if UNITY_2018_3_OR_NEWER - DisposeSentinel.Create(out m_Safety, out m_DisposeSentinel, 0, allocator); + DisposeSentinel.Create(out m_Safety, out m_DisposeSentinel, 0, allocator); #else DisposeSentinel.Create(out m_Safety, out m_DisposeSentinel, 0); #endif @@ -258,6 +271,9 @@ public int this[int index] get { RequireReadAccess(); +#if ENABLE_UNITY_COLLECTIONS_CHECKS + if (index < 0 || index >= Length) throw new ArgumentException("Index must be between 0 and Length - 1.", nameof(index)); +#endif int value = 0; for (int i = 0; i < JobsUtility.MaxJobThreadCount; ++i) { @@ -270,6 +286,9 @@ public int this[int index] set { RequireWriteAccess(); +#if ENABLE_UNITY_COLLECTIONS_CHECKS + if (index < 0 || index >= Length) throw new ArgumentException("Index must be between 0 and Length - 1.", nameof(index)); +#endif *(m_Buffer + index) = value; for (int i = 1; i < JobsUtility.MaxJobThreadCount; ++i) { @@ -291,7 +310,7 @@ public Parallel GetParallel() Parallel parallel = new Parallel(Length, m_Buffer, m_Safety); AtomicSafetyHandle.UseSecondaryVersion(ref parallel.m_Safety); #else - Parallel parallel = new Parallel(m_Buffer); + Parallel parallel = new Parallel(Length, m_Buffer); #endif return parallel; } @@ -339,7 +358,7 @@ public void Dispose() // Make sure we're not double-disposing #if ENABLE_UNITY_COLLECTIONS_CHECKS #if UNITY_2018_3_OR_NEWER - DisposeSentinel.Dispose(ref m_Safety, ref m_DisposeSentinel); + DisposeSentinel.Dispose(ref m_Safety, ref m_DisposeSentinel); #else DisposeSentinel.Dispose(m_Safety, ref m_DisposeSentinel); #endif @@ -373,7 +392,6 @@ public void TestUseOnlySetAllowReadAndWriteAccess( /// Throw an exception if the object isn't readable /// [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] - [BurstDiscard] private void RequireReadAccess() { #if ENABLE_UNITY_COLLECTIONS_CHECKS @@ -385,7 +403,6 @@ private void RequireReadAccess() /// Throw an exception if the object isn't writable /// [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] - [BurstDiscard] private void RequireWriteAccess() { #if ENABLE_UNITY_COLLECTIONS_CHECKS @@ -395,40 +412,38 @@ private void RequireWriteAccess() } /// - /// Provides a debugger view of . + /// Provides a debugger view of . /// internal sealed class NativePerJobThreadIntPtrsDebugView { /// /// The object to provide a debugger view for /// - private NativePerJobThreadIntPtrs ptrs; + private NativePerJobThreadIntPtrs m_Ptrs; /// /// Create the debugger view /// /// - /// + /// /// The object to provide a debugger view for /// public NativePerJobThreadIntPtrsDebugView(NativePerJobThreadIntPtrs ptrs) { - this.ptrs = ptrs; + m_Ptrs = ptrs; } /// - /// Get the viewed object's value - /// - /// - /// - /// The viewed object's value - /// - public int this[int index] - { - get - { - return ptrs[index]; - } - } + /// Get the elements of the array as a managed array + /// + public int[] Items + { + get + { + int[] arr = new int[m_Ptrs.Length]; + for (int i = 0; i < arr.Length; i++) arr[i] = m_Ptrs[i]; + return arr; + } + } } } From 1e6736f5f736af54f6c6eb05a776362271a4ffa5 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 26 Nov 2019 15:11:10 -0500 Subject: [PATCH 3/4] readonly length --- JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs index 3ab3a11..65c0d1b 100644 --- a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs +++ b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs @@ -43,7 +43,7 @@ public struct Parallel /// /// The number of integers stored in the array. /// - public int Length; + public readonly int Length; /// /// Pointers to the integer values in native memory From ee1f6dd6a700b28f9a80e6b8d7e7a620913893be Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 26 Nov 2019 15:13:50 -0500 Subject: [PATCH 4/4] inverse boolean value --- JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs index 65c0d1b..7bb5903 100644 --- a/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs +++ b/JacksonDunstanNativeCollections/NativePerJobThreadIntPtrs.cs @@ -224,7 +224,7 @@ private void RequireWriteAccess() public NativePerJobThreadIntPtrs(int length, Allocator allocator) { // Require a valid allocator - if (UnsafeUtility.IsValidAllocator(allocator)) + if (!UnsafeUtility.IsValidAllocator(allocator)) { throw new ArgumentException( "Allocator must be Temp, TempJob or Persistent",