Feedback Suggestion: Require more explicit declaration of Write access to avoid performance pitfalls

Currently the [ReadOnly] attribute and readonly boolean parameter in Component accessors for jobs are completely optional. We have noticed in our team that this often causes developers to simply forget to specify them, leading to jobs preventing each other from running in parallel due to the default behaviour being that the DOTS framework schedules them with write access. Performance problems due to these issues are currently quite hard to understand and locate, as we can't see the time from scheduling to completion in the profilers, they only show the time elapsed during execution.

In addition to the issue with scheduling jobs, defaulting to write access also causes a bump to the chunk versions, meaning that forgetting to explicitly specify ReadOnly access triggers reactive systems to run when not intended.

I believe it would be far easier to avoid performance pitfalls if the default access behaviour would be ReadOnly unless specified. Additionally, I feel as though the optional parameters in GetArchetypeChunkComponentType(bool readonly = false), GetComponentDataFromEntity(bool readonly = false) and their variants shouldn't be optional, or atleast default in the other way. Not enforcing developers to specify these have caused us many mistakes as well.

For fun, let's make this a poll to see what all you other developers think!

 

It's the entire job of a system to mutate state. What it needs for read-only data is conceptually inconsequential by comparison. You need to think about data access patterns across systems regardless of whether read/write is the one requiring an attribute, so it doesn't really change the mental burden to swap them.

Additionally I highly recommend that you set code standards that the `readonly` parameters always be specified. E.g. `GetComponentDataFromEntity(readonly:true)`.

 

I'd consider it a good solution when the compiler would throw error when trying to write read only component (does it now?). I think explicit [ReadOnly] was chosen, because if you forget that you get worse performance. If you forget [Writable] and write, you can get wrong behaviour.

By the way there's 'in' specifier which might be used instead of 'ref' for readonly. That seems to me like better solution than attribute, because it's enforced by compiler and throws you error on accidental write. However potential issue might be gazzilion of new interfaces with all the combinations (there's already a ton of IJobForeach)

 

If it was compiler enforced, I would prefer the relative few writes stand out in my code compared to the relatively many reads. I wonder if Unity could make it a project setting?