From 7fbfdb07d87bd3b55da154722afa2bff2a0456f8 Mon Sep 17 00:00:00 2001 From: Mike Baker <1426795+mbaker3@users.noreply.github.com> Date: Thu, 20 Jul 2023 22:43:00 -0400 Subject: [PATCH 1/4] CancelRequests - Allow multiple cancel request writers in a single job Add the ability to require multiple cancel streams in a single job. Requiring is done as you would expect, just target the different streams on the JobConfig. Fulfilling the writer/instance is done by passing the stream instance into the `Fulfill` method. The access wrappers now represent access to the cancel type not a specific cancel stream instance. This works because all of the cancel streams share the same SharedWrite Pending collection so getting write access for one stream grants write access for all. The instance passed through `Fulfill` will get safety checked in the access wrapper and delivered back to be used for writer creation. Behaviour: - One cancel request stream required: `jobData.Fulfill` behaves as normal, no explicit instance required. - N cancel request stream required: `jobData.Fulfill` must be provided the instance for each stream to fulfill otherwise the safety system will throw an exception. Note: This is a workaround implementation. #224 will make this cleaner. This approach can be applied to all write wide (shard write) types. Support will be added in an upcoming commit. --- .../Job/JobConfig/AbstractJobConfig.cs | 6 +- .../TaskSet/Job/JobData/AbstractJobData.cs | 8 +- .../Job/Wrapper/AbstractAccessWrapper.cs | 31 ++++++++ .../CancelProgressLookupAccessWrapper.cs | 8 +- .../CancelRequestsPendingAccessWrapper.cs | 75 +++++++++++++++++-- .../Cancellation/CancelRequestsDataStream.cs | 8 +- 6 files changed, 118 insertions(+), 18 deletions(-) diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs index 3411623d..89b4a79e 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs @@ -124,12 +124,14 @@ private void ResolveDuplicateAccessWrappers(AbstractAccessWrapper accessWrapper) // If the existing access wrapper facilitates the needs of the new one then just keep the existing one. if (existingAccessWrapper.AccessType.IsCompatibleWith(accessWrapper.AccessType)) { + existingAccessWrapper.MergeStateFrom(accessWrapper); accessWrapper.Dispose(); } // If the new access wrapper facilitates the needs of the existing one then dispose the existing wrapper and // use the new access wrapper. else if (accessWrapper.AccessType.IsCompatibleWith(existingAccessWrapper.AccessType)) { + accessWrapper.MergeStateFrom(existingAccessWrapper); existingAccessWrapper.Dispose(); m_AccessWrappers[accessWrapper.ID] = accessWrapper; } @@ -438,12 +440,12 @@ CancelProgressLookupAccessWrapper cancelProgressLookupAccessWrapper return cancelProgressLookupAccessWrapper.ProgressLookup; } - internal CancelRequestsDataStream GetCancelRequestsDataStream() + internal CancelRequestsDataStream GetCancelRequestsDataStream(IAbstractCancelRequestDataStream explicitSource = null) { CancelRequestsPendingAccessWrapper cancelRequestsPendingAccessWrapper = GetAccessWrapper(Usage.RequestCancel); - return cancelRequestsPendingAccessWrapper.CancelRequestsDataStream; + return cancelRequestsPendingAccessWrapper.GetInstance(explicitSource); } internal EntityProxyDataStream GetPendingDataStream(Usage usage) diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs index 5b7dd0d7..45c3e189 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs @@ -34,9 +34,9 @@ protected AbstractJobData(IJobConfig jobConfig) /// /// Fulfills an instance of the provided type for the job. /// - public void Fulfill(out CancelRequestsWriter instance) + public void Fulfill(out CancelRequestsWriter instance, IAbstractCancelRequestDataStream explicitSource = null) { - CancelRequestsDataStream cancelRequestDataStream = m_JobConfig.GetCancelRequestsDataStream(); + CancelRequestsDataStream cancelRequestDataStream = m_JobConfig.GetCancelRequestsDataStream(explicitSource); instance = cancelRequestDataStream.CreateCancelRequestsWriter(); } @@ -60,11 +60,11 @@ public void Fulfill(out DataStreamActiveReader instance) instance = dataStream.CreateDataStreamActiveReader(); } - + //************************************************************************************************************* // ENTITY SPAWNER //************************************************************************************************************* - + public void Fulfill(out EntitySpawner entitySpawner) { entitySpawner = m_JobConfig.GetEntitySpawner(); diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs index c3049d79..8a5f728f 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs @@ -1,4 +1,5 @@ using Anvil.CSharp.Core; +using Anvil.CSharp.Logging; using Anvil.Unity.DOTS.Jobs; using System; using System.Diagnostics; @@ -23,6 +24,16 @@ protected AbstractAccessWrapper(AccessType accessType, AbstractJobConfig.Usage u public abstract JobHandle AcquireAsync(); public abstract void ReleaseAsync(JobHandle dependsOn); + /// + /// Merge the state from another wrapper instance of the same type + /// + /// + public virtual void MergeStateFrom(AbstractAccessWrapper other) + { + Debug_EnsureSameID(other); + Debug_EnsureSameType(other); + } + //************************************************************************************************************* // SAFETY //************************************************************************************************************* @@ -38,5 +49,25 @@ private void Debug_SetWrapperType() ? type.GetGenericTypeDefinition() : type; } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void Debug_EnsureSameID(AbstractAccessWrapper wrapper) + { + if (ID != wrapper.ID) + { + throw new Exception($"Cannot merge two wrappers with different IDs. This:{ID}, Other:{wrapper.ID}"); + } + } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void Debug_EnsureSameType(AbstractAccessWrapper wrapper) + { + Type thisType = GetType(); + Type otherType = wrapper.GetType(); + if (thisType != otherType) + { + throw new Exception($"Cannot merge two wrappers of different types. This:{thisType.GetReadableName()}, Other:{otherType.GetReadableName()}"); + } + } } } \ No newline at end of file diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelProgressLookupAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelProgressLookupAccessWrapper.cs index c3730a95..c2153879 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelProgressLookupAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelProgressLookupAccessWrapper.cs @@ -11,7 +11,11 @@ internal class CancelProgressLookupAccessWrapper : AbstractAccessWrapper public UnsafeParallelHashMap ProgressLookup { get; } - public CancelProgressLookupAccessWrapper(ActiveLookupData cancelProgressLookupData, AccessType accessType, AbstractJobConfig.Usage usage) : base(accessType, usage) + public CancelProgressLookupAccessWrapper( + ActiveLookupData cancelProgressLookupData, + AccessType accessType, + AbstractJobConfig.Usage usage) + : base(accessType, usage) { m_CancelProgressLookupData = cancelProgressLookupData; ProgressLookup = m_CancelProgressLookupData.Lookup; @@ -27,4 +31,4 @@ public override void ReleaseAsync(JobHandle dependsOn) m_CancelProgressLookupData.ReleaseAsync(dependsOn); } } -} +} \ No newline at end of file diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs index 0866d6c5..85a9e6d5 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs @@ -1,29 +1,92 @@ +using Anvil.CSharp.Logging; using Anvil.Unity.DOTS.Jobs; +using System; +using System.Collections.Generic; +using System.Diagnostics; using Unity.Jobs; namespace Anvil.Unity.DOTS.Entities.TaskDriver { internal class CancelRequestsPendingAccessWrapper : AbstractAccessWrapper { - public CancelRequestsDataStream CancelRequestsDataStream { get; } + private readonly CancelRequestsDataStream m_DefaultStream; public CancelRequestsPendingAccessWrapper( - CancelRequestsDataStream cancelRequestsDataStream, + CancelRequestsDataStream defaultStream, AccessType accessType, AbstractJobConfig.Usage usage) : base(accessType, usage) { - CancelRequestsDataStream = cancelRequestsDataStream; + m_DefaultStream = defaultStream; + DEBUG_TrackRequiredStream(defaultStream); } public override JobHandle AcquireAsync() { - return CancelRequestsDataStream.AcquirePendingAsync(AccessType); + return m_DefaultStream.AcquirePendingAsync(AccessType); } public override void ReleaseAsync(JobHandle dependsOn) { - CancelRequestsDataStream.ReleasePendingAsync(dependsOn); + m_DefaultStream.ReleasePendingAsync(dependsOn); + } + + public CancelRequestsDataStream GetInstance(IAbstractCancelRequestDataStream explicitStream = null) + { + DEBUG_EnforceExplicitStream(explicitStream); + DEBUG_EnsureStreamWasRequired(explicitStream); + + return (CancelRequestsDataStream)explicitStream ?? m_DefaultStream; + } + + public override void MergeStateFrom(AbstractAccessWrapper other) + { + base.MergeStateFrom(other); + + CancelRequestsPendingAccessWrapper otherTyped = (CancelRequestsPendingAccessWrapper)other; + m_DEBUG_RequiredStreams.UnionWith(otherTyped.m_DEBUG_RequiredStreams); + DEBUG_TrackRequiredStream(otherTyped.m_DefaultStream); + } + + //************************************************************************************************************* + // SAFETY + //************************************************************************************************************* + + // #if ANVIL_DEBUG_SAFETY + private HashSet m_DEBUG_RequiredStreams; + + [Conditional("ANVIL_DEBUG_SAFETY")] + public void DEBUG_TrackRequiredStream(CancelRequestsDataStream stream) + { + m_DEBUG_RequiredStreams ??= new HashSet(1); + m_DEBUG_RequiredStreams.Add(stream); + } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void DEBUG_EnsureStreamWasRequired(IAbstractCancelRequestDataStream stream) + { + CancelRequestsDataStream cancelStream = (CancelRequestsDataStream)stream; + if (stream == null || m_DEBUG_RequiredStreams.Contains(cancelStream)) + { + return; + } + + throw new Exception($"The explicit stream instance requested was not set as required. DataTargetID:{cancelStream.DataTargetID}"); + } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void DEBUG_EnforceExplicitStream(IAbstractCancelRequestDataStream stream) + { + int requiredStreamCount = m_DEBUG_RequiredStreams.Count; + if (stream == null && requiredStreamCount > 1) + { + throw new Exception($"More than one stream has set this type as a requirement. The exact stream must be provided on retrieval. Type:{typeof(CancelRequestsDataStream).GetReadableName()}"); + } + + if (stream != null && requiredStreamCount == 1) + { + Logger.Warning($"An explicit stream was provided when not required. Consider using default fulfillment. Type:{typeof(CancelRequestsDataStream).GetReadableName()}"); + } } } -} +} \ No newline at end of file diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/TaskData/DataStream/Cancellation/CancelRequestsDataStream.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/TaskData/DataStream/Cancellation/CancelRequestsDataStream.cs index a0cd1590..e62bebec 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/TaskData/DataStream/Cancellation/CancelRequestsDataStream.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/TaskData/DataStream/Cancellation/CancelRequestsDataStream.cs @@ -9,7 +9,7 @@ internal class CancelRequestsDataStream : AbstractDataStream, ISystemCancelRequestDataStream { private const string UNIQUE_CONTEXT_IDENTIFIER = "CANCEL_REQUEST"; - + private readonly CancelRequestsDataSource m_DataSource; public ActiveLookupData ActiveLookupData { get; } @@ -22,7 +22,7 @@ public override IDataSource DataSource { get => m_DataSource; } - + /// public uint ActiveDataVersion { @@ -88,11 +88,11 @@ public void ReleaseCancelRequestsWriter() { ReleasePending(); } - + /// public bool IsActiveDataInvalidated(uint lastVersion) { return ActiveLookupData.IsDataInvalidated(lastVersion); } } -} +} \ No newline at end of file From 9239df7736367bceeb6c33994b63cdd848314f46 Mon Sep 17 00:00:00 2001 From: Mike Baker <1426795+mbaker3@users.noreply.github.com> Date: Fri, 21 Jul 2023 16:18:48 -0400 Subject: [PATCH 2/4] Resolve #245 - Multiple Streams of the Same Type in the same job --- .../Job/JobConfig/AbstractJobConfig.cs | 8 +- .../TaskSet/Job/JobData/AbstractJobData.cs | 4 +- ... AbstractDataStreamActiveAccessWrapper.cs} | 4 +- ...ractDataStreamActiveAccessWrapper.cs.meta} | 0 .../AbstractDataStreamPendingAccessWrapper.cs | 79 +++++++++++++++++++ ...ractDataStreamPendingAccessWrapper.cs.meta | 3 + .../CancelRequestsPendingAccessWrapper.cs | 72 +---------------- .../Wrapper/DataStreamActiveAccessWrapper.cs | 2 +- .../DataStreamActiveCancelAccessWrapper.cs | 4 +- .../Wrapper/DataStreamPendingAccessWrapper.cs | 6 +- 10 files changed, 99 insertions(+), 83 deletions(-) rename Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/{AbstractDataStreamAccessWrapper.cs => AbstractDataStreamActiveAccessWrapper.cs} (55%) rename Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/{AbstractDataStreamAccessWrapper.cs.meta => AbstractDataStreamActiveAccessWrapper.cs.meta} (100%) create mode 100644 Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs create mode 100644 Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs.meta diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs index 89b4a79e..27c8f280 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobConfig/AbstractJobConfig.cs @@ -448,13 +448,15 @@ CancelRequestsPendingAccessWrapper cancelRequestsPendingAccessWrapper return cancelRequestsPendingAccessWrapper.GetInstance(explicitSource); } - internal EntityProxyDataStream GetPendingDataStream(Usage usage) + internal EntityProxyDataStream GetPendingDataStream( + Usage usage, + IAbstractDataStream explicitSource = null) where TInstance : unmanaged, IEntityKeyedTask { DataStreamPendingAccessWrapper dataStreamAccessWrapper = GetAccessWrapper>(usage); - return dataStreamAccessWrapper.DataStream; + return dataStreamAccessWrapper.GetInstance(explicitSource); } internal EntityProxyDataStream GetActiveDataStream(Usage usage) @@ -619,7 +621,7 @@ private void Debug_EnsureWrapperExists(JobConfigDataID id) [Conditional("ANVIL_DEBUG_SAFETY")] private void Debug_EnsureWrapperUsage(AbstractAccessWrapper wrapper) { - if (wrapper.Debug_WrapperType != typeof(AbstractDataStreamAccessWrapper<>)) + if (wrapper.Debug_WrapperType != typeof(AbstractDataStreamActiveAccessWrapper<>)) { return; } diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs index 45c3e189..f74b7cfd 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/JobData/AbstractJobData.cs @@ -43,10 +43,10 @@ public void Fulfill(out CancelRequestsWriter instance, IAbstractCancelRequestDat /// /// Fulfills an instance of the provided type for the job. /// - public void Fulfill(out DataStreamPendingWriter instance) + public void Fulfill(out DataStreamPendingWriter instance, IAbstractDataStream explicitSource = null) where TInstance : unmanaged, IEntityKeyedTask { - EntityProxyDataStream dataStream = m_JobConfig.GetPendingDataStream(AbstractJobConfig.Usage.Default); + EntityProxyDataStream dataStream = m_JobConfig.GetPendingDataStream(AbstractJobConfig.Usage.Default, explicitSource); instance = dataStream.CreateDataStreamPendingWriter(); } diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamActiveAccessWrapper.cs similarity index 55% rename from Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamAccessWrapper.cs rename to Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamActiveAccessWrapper.cs index cf969e6f..0d985ca9 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamActiveAccessWrapper.cs @@ -2,12 +2,12 @@ namespace Anvil.Unity.DOTS.Entities.TaskDriver { - internal abstract class AbstractDataStreamAccessWrapper : AbstractAccessWrapper + internal abstract class AbstractDataStreamActiveAccessWrapper : AbstractAccessWrapper where T : unmanaged, IEntityKeyedTask { public EntityProxyDataStream DataStream { get; } - protected AbstractDataStreamAccessWrapper(EntityProxyDataStream dataStream, AccessType accessType, AbstractJobConfig.Usage usage) + protected AbstractDataStreamActiveAccessWrapper(EntityProxyDataStream dataStream, AccessType accessType, AbstractJobConfig.Usage usage) : base(accessType, usage) { DataStream = dataStream; diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamAccessWrapper.cs.meta b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamActiveAccessWrapper.cs.meta similarity index 100% rename from Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamAccessWrapper.cs.meta rename to Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamActiveAccessWrapper.cs.meta diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs new file mode 100644 index 00000000..59c640c9 --- /dev/null +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs @@ -0,0 +1,79 @@ +using Anvil.CSharp.Logging; +using Anvil.Unity.DOTS.Jobs; +using System; +using System.Collections.Generic; +using System.Diagnostics; + +namespace Anvil.Unity.DOTS.Entities.TaskDriver +{ + internal abstract class AbstractDataStreamPendingAccessWrapper : AbstractAccessWrapper + where T : AbstractDataStream, IAbstractDataStream + { + protected readonly T m_DefaultStream; + + protected AbstractDataStreamPendingAccessWrapper(T defaultStream, AccessType accessType, AbstractJobConfig.Usage usage) + : base(accessType, usage) + { + m_DefaultStream = defaultStream; + DEBUG_TrackRequiredStream(defaultStream); + } + + public T GetInstance(IAbstractDataStream explicitStream = null) + { + DEBUG_EnforceExplicitStream(explicitStream); + DEBUG_EnsureStreamWasRequired(explicitStream); + + return (T)explicitStream ?? m_DefaultStream; + } + + public override void MergeStateFrom(AbstractAccessWrapper other) + { + base.MergeStateFrom(other); + + AbstractDataStreamPendingAccessWrapper otherTyped = (AbstractDataStreamPendingAccessWrapper)other; + m_DEBUG_RequiredStreams.UnionWith(otherTyped.m_DEBUG_RequiredStreams); + DEBUG_TrackRequiredStream(otherTyped.m_DefaultStream); + } + + //************************************************************************************************************* + // SAFETY + //************************************************************************************************************* + + // #if ANVIL_DEBUG_SAFETY + private HashSet m_DEBUG_RequiredStreams; + + [Conditional("ANVIL_DEBUG_SAFETY")] + public void DEBUG_TrackRequiredStream(T stream) + { + m_DEBUG_RequiredStreams ??= new HashSet(1); + m_DEBUG_RequiredStreams.Add(stream); + } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void DEBUG_EnsureStreamWasRequired(IAbstractDataStream stream) + { + T typedStream = (T)stream; + if (stream == null || m_DEBUG_RequiredStreams.Contains(typedStream)) + { + return; + } + + throw new Exception($"The explicit stream instance requested was not set as required. DataTargetID:{typedStream.DataTargetID}"); + } + + [Conditional("ANVIL_DEBUG_SAFETY")] + private void DEBUG_EnforceExplicitStream(IAbstractDataStream stream) + { + int requiredStreamCount = m_DEBUG_RequiredStreams.Count; + if (stream == null && requiredStreamCount > 1) + { + throw new Exception($"More than one stream has set this type as a requirement. The exact stream must be provided on retrieval. Type:{typeof(T).GetReadableName()}"); + } + + if (stream != null && requiredStreamCount == 1) + { + Logger.Warning($"An explicit stream was provided when not required. Consider using default fulfillment. Type:{typeof(T).GetReadableName()}"); + } + } + } +} diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs.meta b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs.meta new file mode 100644 index 00000000..4408e794 --- /dev/null +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: d9315f70208943239942c01ea7639aee +timeCreated: 1689970623 \ No newline at end of file diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs index 85a9e6d5..18f26ecc 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/CancelRequestsPendingAccessWrapper.cs @@ -1,25 +1,15 @@ -using Anvil.CSharp.Logging; using Anvil.Unity.DOTS.Jobs; -using System; -using System.Collections.Generic; -using System.Diagnostics; using Unity.Jobs; namespace Anvil.Unity.DOTS.Entities.TaskDriver { - internal class CancelRequestsPendingAccessWrapper : AbstractAccessWrapper + internal class CancelRequestsPendingAccessWrapper : AbstractDataStreamPendingAccessWrapper { - private readonly CancelRequestsDataStream m_DefaultStream; - public CancelRequestsPendingAccessWrapper( CancelRequestsDataStream defaultStream, AccessType accessType, AbstractJobConfig.Usage usage) - : base(accessType, usage) - { - m_DefaultStream = defaultStream; - DEBUG_TrackRequiredStream(defaultStream); - } + : base(defaultStream, accessType, usage) { } public override JobHandle AcquireAsync() { @@ -30,63 +20,5 @@ public override void ReleaseAsync(JobHandle dependsOn) { m_DefaultStream.ReleasePendingAsync(dependsOn); } - - public CancelRequestsDataStream GetInstance(IAbstractCancelRequestDataStream explicitStream = null) - { - DEBUG_EnforceExplicitStream(explicitStream); - DEBUG_EnsureStreamWasRequired(explicitStream); - - return (CancelRequestsDataStream)explicitStream ?? m_DefaultStream; - } - - public override void MergeStateFrom(AbstractAccessWrapper other) - { - base.MergeStateFrom(other); - - CancelRequestsPendingAccessWrapper otherTyped = (CancelRequestsPendingAccessWrapper)other; - m_DEBUG_RequiredStreams.UnionWith(otherTyped.m_DEBUG_RequiredStreams); - DEBUG_TrackRequiredStream(otherTyped.m_DefaultStream); - } - - //************************************************************************************************************* - // SAFETY - //************************************************************************************************************* - - // #if ANVIL_DEBUG_SAFETY - private HashSet m_DEBUG_RequiredStreams; - - [Conditional("ANVIL_DEBUG_SAFETY")] - public void DEBUG_TrackRequiredStream(CancelRequestsDataStream stream) - { - m_DEBUG_RequiredStreams ??= new HashSet(1); - m_DEBUG_RequiredStreams.Add(stream); - } - - [Conditional("ANVIL_DEBUG_SAFETY")] - private void DEBUG_EnsureStreamWasRequired(IAbstractCancelRequestDataStream stream) - { - CancelRequestsDataStream cancelStream = (CancelRequestsDataStream)stream; - if (stream == null || m_DEBUG_RequiredStreams.Contains(cancelStream)) - { - return; - } - - throw new Exception($"The explicit stream instance requested was not set as required. DataTargetID:{cancelStream.DataTargetID}"); - } - - [Conditional("ANVIL_DEBUG_SAFETY")] - private void DEBUG_EnforceExplicitStream(IAbstractCancelRequestDataStream stream) - { - int requiredStreamCount = m_DEBUG_RequiredStreams.Count; - if (stream == null && requiredStreamCount > 1) - { - throw new Exception($"More than one stream has set this type as a requirement. The exact stream must be provided on retrieval. Type:{typeof(CancelRequestsDataStream).GetReadableName()}"); - } - - if (stream != null && requiredStreamCount == 1) - { - Logger.Warning($"An explicit stream was provided when not required. Consider using default fulfillment. Type:{typeof(CancelRequestsDataStream).GetReadableName()}"); - } - } } } \ No newline at end of file diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveAccessWrapper.cs index 269cd316..4a3d3d69 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveAccessWrapper.cs @@ -5,7 +5,7 @@ namespace Anvil.Unity.DOTS.Entities.TaskDriver { //TODO: Can we simplify this a lot and get rid of a bunch of special Access Wrapper types? //TODO: https://github.com/decline-cookies/anvil-unity-dots/pull/105#discussion_r1043593841 - internal class DataStreamActiveAccessWrapper : AbstractDataStreamAccessWrapper + internal class DataStreamActiveAccessWrapper : AbstractDataStreamActiveAccessWrapper where T : unmanaged, IEntityKeyedTask { public DataStreamActiveAccessWrapper( diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveCancelAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveCancelAccessWrapper.cs index 05f1de77..102c181f 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveCancelAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamActiveCancelAccessWrapper.cs @@ -3,7 +3,7 @@ namespace Anvil.Unity.DOTS.Entities.TaskDriver { - internal class DataStreamActiveCancelAccessWrapper : AbstractDataStreamAccessWrapper, + internal class DataStreamActiveCancelAccessWrapper : AbstractDataStreamActiveAccessWrapper, IDataStreamPendingAccessWrapper where T : unmanaged, IEntityKeyedTask { @@ -19,4 +19,4 @@ public override void ReleaseAsync(JobHandle dependsOn) DataStream.ReleaseActiveCancelAsync(dependsOn); } } -} \ No newline at end of file +} diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamPendingAccessWrapper.cs index 9d2e0d7f..c5c9858a 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamPendingAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/DataStreamPendingAccessWrapper.cs @@ -3,7 +3,7 @@ namespace Anvil.Unity.DOTS.Entities.TaskDriver { - internal class DataStreamPendingAccessWrapper : AbstractDataStreamAccessWrapper, IDataStreamPendingAccessWrapper + internal class DataStreamPendingAccessWrapper : AbstractDataStreamPendingAccessWrapper>, IDataStreamPendingAccessWrapper where T : unmanaged, IEntityKeyedTask { public DataStreamPendingAccessWrapper(EntityProxyDataStream dataStream, AccessType accessType, AbstractJobConfig.Usage usage) @@ -11,12 +11,12 @@ public DataStreamPendingAccessWrapper(EntityProxyDataStream dataStream, Acces public override JobHandle AcquireAsync() { - return DataStream.AcquirePendingAsync(AccessType); + return m_DefaultStream.AcquirePendingAsync(AccessType); } public override void ReleaseAsync(JobHandle dependsOn) { - DataStream.ReleasePendingAsync(dependsOn); + m_DefaultStream.ReleasePendingAsync(dependsOn); } } } \ No newline at end of file From c5aaa91974b3868774b08651e998ca069d2b5b12 Mon Sep 17 00:00:00 2001 From: Mike Baker <1426795+mbaker3@users.noreply.github.com> Date: Tue, 1 Aug 2023 09:59:20 -0400 Subject: [PATCH 3/4] AbstractAccessWrapper - Docs + Cleanup --- .../TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs | 2 +- .../Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs index 8a5f728f..d349f5d3 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractAccessWrapper.cs @@ -27,7 +27,7 @@ protected AbstractAccessWrapper(AccessType accessType, AbstractJobConfig.Usage u /// /// Merge the state from another wrapper instance of the same type /// - /// + /// The wrapper instance to read state from. public virtual void MergeStateFrom(AbstractAccessWrapper other) { Debug_EnsureSameID(other); diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs index 59c640c9..c2dc9486 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs @@ -43,7 +43,7 @@ public override void MergeStateFrom(AbstractAccessWrapper other) private HashSet m_DEBUG_RequiredStreams; [Conditional("ANVIL_DEBUG_SAFETY")] - public void DEBUG_TrackRequiredStream(T stream) + private void DEBUG_TrackRequiredStream(T stream) { m_DEBUG_RequiredStreams ??= new HashSet(1); m_DEBUG_RequiredStreams.Add(stream); @@ -76,4 +76,4 @@ private void DEBUG_EnforceExplicitStream(IAbstractDataStream stream) } } } -} +} \ No newline at end of file From b7c4de80480f367c59845cffd00546da5609c67f Mon Sep 17 00:00:00 2001 From: Mike Baker <1426795+mbaker3@users.noreply.github.com> Date: Tue, 1 Aug 2023 10:04:46 -0400 Subject: [PATCH 4/4] AbstractAccessWrapper - Docs + Cleanup --- .../Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs index c2dc9486..8847e6ad 100644 --- a/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs +++ b/Scripts/Runtime/Entities/TaskDriver/TaskSet/Job/Wrapper/AbstractDataStreamPendingAccessWrapper.cs @@ -39,7 +39,7 @@ public override void MergeStateFrom(AbstractAccessWrapper other) // SAFETY //************************************************************************************************************* - // #if ANVIL_DEBUG_SAFETY + // Used only with #if ANVIL_DEBUG_SAFETY private HashSet m_DEBUG_RequiredStreams; [Conditional("ANVIL_DEBUG_SAFETY")]