2

So I'm designing a data type to use as the cache key in a memory cache. There will be millions of entries for this cache and I'm wondering which of a class or struct would be most appropriate for this purpose any why. I have read the MS guidelines on this matter and they suggest:

It logically represents a single value, similar to primitive types (int, double, etc.).

I think it does.

It has an instance size under 16 bytes.

No, since the guid alone is 16 bytes

It is immutable.

Yes

It will not have to be boxed frequently.

Yes

As it currently looks:

    internal class ProductKey : IEquatable<ProductKey>
    {
        private Guid MerchantId { get; }
        private string ProductId { get; }

        public ProductKey(Guid merchantId, string productId)
        {
            MerchantId = merchantId;
            ProductId = productId;
        }

        public bool Equals(ProductKey other)
        {
            if (ReferenceEquals(null, other)) return false;
            if (ReferenceEquals(this, other)) return true;
            return
                string.Equals(ProductId, other.ProductId, StringComparison.OrdinalIgnoreCase) &&
                MerchantId.Equals(other.MerchantId);
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            if (ReferenceEquals(this, obj)) return true;
            if (obj.GetType() != this.GetType()) return false;
            return Equals((ProductKey)obj);
        }

        public override int GetHashCode()
        {
            var hashCode = new HashCode();
            hashCode.Add(MerchantId);
            hashCode.Add(ProductId, StringComparer.OrdinalIgnoreCase);
            return hashCode.ToHashCode();
        }
    }
Arne
  • 21
  • 2
  • You can simplify `GetHashCode` by using tuples: `int GetHashCode() => (MerchantId, ProductId).GetHashCode()` – Sean Apr 02 '20 at 10:02
  • @Sean How about the `OrdinalIgnoreCase`? – Arne Apr 02 '20 at 10:03
  • @Ame - Just `ToLower` the string – Sean Apr 02 '20 at 10:04
  • 2
    @Sean1 ToLower is not correct in this case for three reasons: first, Microsoft Guidelines prefer ToUpper as there can be issues in converting the characters from upper case to lower case. Second, it allocates memory (a new string) which could be a performance hit. Finally, ToLower uses the culture of the user and you would never want to use it for something that is not presented in UI. The use of StringComparer allows to be culture insensitive (they are using Ordinal) and do not allocate memory for the comparison. – Yennefer Apr 02 '20 at 10:11

1 Answers1

1

Struct usage greatly depends on the whole environment which they will be used. It is not just simply that uses less memory, it has a change in semantics that is non trivial too.

Structs are marshalled by value, that is, they are copied. Classes are marshalled by reference.

C# 7 and 8 introduce many optimizations for structures, nonetheless, if used incorrectly, they could cause more harm than good.

There already is a long-standing debate about this hot topic. You may want to take a look at this SO question: you'll see that there are many different scenarios, with different ideas.

Just a note for your particular case, your structure holds a reference, therefore you'll save memory when your structure is copied because you only have one integer and a pointer to marshal, but you won't gain any performance in accessing the string, as it is allocated elsewhere. This means that code locality won't improve.

Yennefer
  • 5,704
  • 7
  • 31
  • 44
  • So for my particular case (as key in a memory cache, that could be longed lived) what would you suggest? – Arne Apr 02 '20 at 11:31
  • Personally, I would go for a structure as long as it can be stored in a typed fashion (e.g. Dictionary. However, I would first implement the software requirements without thinking at optimizations. Then, I would look whether or not an optimization is required. Optimized or convoluted code is not easy maintainable. – Yennefer Apr 02 '20 at 13:03
  • The key in `Microsoft.Extensions.Caching.Memory.MemoryCache` is `object` so I suppose it would be boxed. – Arne Apr 02 '20 at 13:21
  • Yes, it will be boxed. As you need an object, you can create a key as a string by concatenating the two fields: for example ```var key = MerchantId.ToString()+ "#" + ProductId.ToUpperInvariant()``` this way the key is already good enough for being used in the cache and you are not creating (and maintaining) an ad-hoc class for implementng the key. – Yennefer Apr 02 '20 at 13:44