search

decline-cookies / anvil-unity-dots

Unity DOTS and ECS specific additions and extensions to Anvil
MIT License
4 stars 1 forks source link

Access Wrapper Fix #160

Closed jkeon closed 1 year ago

jkeon commented 1 year ago

Fixing an Acquire/Release error when the same job requires writing to two or more DataStreams of the same type.

What is the current behaviour?

AccessWrappers are created for the type and given the context of the instance that owns them. When a job is scheduled, we loop through all the wrappers and AcquireAsync to ensure we have access to them. We then schedule the job and then loop through all the wrappers again and ReleaseAsync to complete the cycle.

Because DataStreams share a DataSource all Write operations operate on the same underlying collection regardless of the instance that will use it.

Ex. TaskDriverA with DataStream and TaskDriverB with DataStream will both write to a DataSource. Upon that DataSource's consolidation, the active instances will be written to the correct TaskDriver's instance of DataStream.

This results in two acquires in a row to the same underlying AccessController.

What is the new behaviour?

When hardening the JobConfig, we only add the unique wrappers.

What issues does this resolve?

What PRs does this depend on?

Does this introduce a breaking change?

jkeon commented 1 year ago

In addition to my one comment. Do these config objects live beyond the hardening stage?

If so, is the HashSet required after hardening? Seems a bit wasteful to keep it around if it's not used afterwards. Not saying we fix now but just plan to some day as part of an optimization or hardening cleanup pass.

The config objects live on, but you're right, that HashSet isn't needed. Removed.