Why is it okay that this struct is mutable? When are mutable structs acceptable?

Question

Eric Lippert told me I should "try to always make value types immutable", so I figured I should try to always make value types immutable.

But, I just found this internal mutable struct, System.Web.Util.SimpleBitVector32, in the System.Web assembly, which makes me think that there must be a good reason for having a mutable struct. I'm guessing the reason that they did it this way is because it performed better under testing, and they kept it internal to discourage its misuse. However, that's speculation.

I've C&P'd the source of this struct. What is it that justifies the design decision to use a mutable struct? In general, what sort of benefits can be gained by the approach and when are these benefits significant enough to justify the potential detriments?

[Serializable, StructLayout(LayoutKind.Sequential)] internal struct SimpleBitVector32 {     private int data;     internal SimpleBitVector32(int data)     {         this.data = data;     }      internal int IntegerValue     {         get { return this.data; }         set { this.data = value; }     }      internal bool this[int bit]     {         get {              return ((this.data & bit) == bit);          }         set {             int data = this.data;             if (value) this.data = data | bit;             else this.data = data & ~bit;         }     }      internal int this[int mask, int offset]     {         get { return ((this.data & mask) >> offset); }         set { this.data = (this.data & ~mask) | (value << offset); }     }      internal void Set(int bit)     {         this.data |= bit;     }      internal void Clear(int bit)     {         this.data &= ~bit;     } } 

Answer 1

Given that the payload is a 32-bit integer, I'd say this could easily have been written as an immutable struct, probably with no impact on performance. Whether you're calling a mutator method that changes the value of a 32-bit field, or replacing a 32-bit struct with a new 32-bit struct, you're still doing the exact same memory operations.

Probably somebody wanted something that acted kind of like an array (while really just being bits in a 32-bit integer), so they decided they wanted to use indexer syntax with it, instead of a less-obvious .WithTheseBitsChanged() method that returns a new struct. Since it wasn't going to be used directly by anyone outside MS's web team, and probably not by very many people even within the web team, I imagine they had quite a bit more leeway in design decisions than the people building the public APIs.

So, no, probably not that way for performance -- it was probably just some programmer's personal preference in coding style, and there was never any compelling reason to change it.

If you're looking for design guidelines, I wouldn't spend too much time looking at code that hasn't been polished for public consumption.

Answer 2

Actually, if you search for all classes containing BitVector in the .NET framework, you'll find a bunch of these beasts :-)

• System.Collections.Specialized.BitVector32 (the sole public one...)
• System.Web.Util.SafeBitVector32 (thread safe)
• System.Web.Util.SimpleBitVector32
• System.Runtime.Caching.SafeBitVector32 (thread safe)
• System.Configuration.SafeBitVector32 (thread safe)
• System.Configuration.SimpleBitVector32

And if you look here were resides the SSCLI (Microsoft Shared Source CLI, aka ROTOR) source of System.Configuration.SimpleBitVector32, you'll find this comment:

// // This is a cut down copy of System.Collections.Specialized.BitVector32. The // reason this is here is because it is used rather intensively by Control and // WebControl. As a result, being able to inline this operations results in a // measurable performance gain, at the expense of some maintainability. // [Serializable()] internal struct SimpleBitVector32 

I believe this says it all. I think the System.Web.Util one is more elaborate but built on the same grounds.