DequeNET
DequeNET copied to clipboard
performance / pragma warning - does _anchor need to be volatile when all exchanges are interlocked?
Although it is possible to perform an Interlocked.Exchange / Interlocked.CompareExchange on a volatile field, and as Microsoft notes, there are cases like this where the warning can be disabled.
However, my understanding is that, as long as all updates to the field are interlocked, the volatile modifier is not needed. The call to Interlocked.*Exchange will ensure that cache is flushed and that subsequent reads will get the updated value, as described in this answer on StackOverflow.
Perhaps there are certain cases where this is necessary; e.g. if you cannot guarantee that all updates will be interlocked, you might mark it as volatile just to be on the safe side. But that does not seem to be relevant for a private field like _anchor.
To improve efficiency and also remove the need for the pragma warning disable, I suggest removing the volatile modifier from _anchor. Or, if there is a reason why it is still required, for performance or otherwise, please advise.
It would be necessary to change 4 places where there is a direct assignment to _anchor, one in the constructor, two in InitializeFromCollection, and one in Clear, to use Interlocked.Exchange(ref _anchor, new Anchor(...)) instead. Beyond that it would also be advisable to mark _anchor as private rather than internal, to ensure that there is no uncontrolled access or direct writes to the field.