StringComparer.CurrentCultureIgnoreCase efficiency with multiple calls in .NET

Question

I've been using StringComparer.CurrentCultureIgnoreCase for case-insensitive comparisons and hashing. But after checking the Reference Source I see it creates a new instance with every call (shouldn't it be a static function then? Just for form's sake). Anyway my question is, when you have multiple comparisons to do, like an IEquality<T> implementation, is it efficient to do:

// 2 instances per call
return StringComparer.CurrentCultureIgnoreCase.Equals(this.a, other.a)
  && StringComparer.CurrentCultureIgnoreCase.Equals(this.b, other.b) .. etc ..

Or maybe:

public bool Equals(MyObj other)
{
  // 1 instance per call
  var equ = StringComparer.CurrentCultureIgnoreCase;
  return equ.Equals(this.a, other.a)
    && equ.Equals(this.b, other.b) .. etc ..
}

Or even cache/pool the comparers so they arn't created every time Equals() is called?

// 1 instance per thread
[ThreadStatic]
private static StringComparer equ;

public bool Equals(MyObj other)
{
  if (equ == null) equ = StringComparer.CurrentCultureIgnoreCase;

  return equ.Equals(this.a, other.a)
    && equ.Equals(this.b, other.b) .. etc ..
}

Any feelings on which is best practice?

(Thanks to michael-liu for pointing out by original reference to OrdinalIgnoreCase is not a new instance, I've switched to CurrentCultureIgnoreCase which is)

Here, you should use `string.Equals(a, b, StringComparison.X)`. — usr, Jun 06 '15 at 15:10
"But after checking the Reference Source I see it creates a new instance with every call" -- This is contradicted by the reference source, linked to in Michael Liu's answer. It would be helpful if you could edit your question to say specifically what you found, and where. — , Jun 06 '15 at 15:22
Thanks @hvd, edited the question. That will teach me not to 'simplify' my questions, and therefore invalidate my question! — user826840, Jun 06 '15 at 15:44

Michael Liu · Answer 1 · 2015-06-06T15:12:41.577

According to the reference source, OrdinalIgnoreCase returns the same static instance each time:

public abstract class StringComparer : ...
{
    ...

    private static readonly StringComparer _ordinalIgnoreCase = new OrdinalComparer(true);        

    ...

    public static StringComparer OrdinalIgnoreCase { 
        get {
            Contract.Ensures(Contract.Result<StringComparer>() != null);
            return _ordinalIgnoreCase;
        }
    }

Since the Contract.Ensures call is omitted in the actual .NET redistributables, the remaining field access will almost certainly be inlined by the jitter.

(The same applies to InvariantCulture, InvariantCultureIgnoreCase, and Ordinal.)

On the other hand, CurrentCulture and CurrentCultureIgnoreCase do return new instances each time you access them because the current culture may change between accesses. Should you cache the comparer in this case? Personally, I wouldn't make my code more complicated unless profiling indicated there was a problem.

In this particular case, though, I usually compare strings for equality like this:

return String.Equals(this.a, other.a, StringComparison.OrdinalIgnoreCase);

Now you don't have to worry about StringComparer allocations at all, even if you use CurrentCulture or CurrentCultureIgnoreCase, and the code is still straightforward to read.

Thanks, I did start off by using `string.Equals(string, string, StringComparison)` but how do you implement GetHashCode with the same comparison rules? — user826840, Jun 06 '15 at 15:25
Even if you use String.Equals and StringComparison to implement equality, you can still use the corresponding StringComparer instance to calculate hash codes. — Michael Liu, Jun 06 '15 at 15:29
Both `CultureAwareComparer.Equals(string, string)` and `GetHashCode(string)` look thread-safe, at least until they disappear into external code. — user826840, Jun 06 '15 at 15:59

score 2 · Answer 2 · answered Jun 06 '15 at 17:14

2

Never underestimate the cost of making code thread-safe. CurrentCulture is a property of a thread and of course different threads can run with different cultures. You'd need a cache that can be accessed in a thread-safe way to store the objects. A cache without a retirement policy is a memory leak, now you also have to keep track of last-usage and a way to retire objects that haven't been used for a while. None is obvious.

Just much simpler and cheaper to create the object when needed. It is quite small, cheaper than a string. It is very unlikely to last long. Memory allocated from gen #0 that doesn't get promoted is very cheap.

The .NET Framework is heavily micro-optimized, they didn't fumble this one.

answered Jun 06 '15 at 17:14

Hans Passant

922,412
146
1,693
2,536

Interesting that `OrdinalComparer` is implicitly thread safe (being only a single instance). But it's not guaranteed by the docs. A quick look at `CultureAwareComparer` looks safe too. – user826840 Jun 06 '15 at 19:28
A comparer is always thread-safe, it just compares without modifying its state. The trouble is CurrentCulture. – Hans Passant Jun 06 '15 at 20:03
you mean currentculture might change, or it might be intrinsicly un-threadsafe? – user826840 Jun 06 '15 at 20:29
Keeping a singleton that depends on CurrentCulture is intrinsically thread-unsafe. Thread-safety is not a transitive property. A class being thread-safe does not make code that uses the class thread-safe. – Hans Passant Jun 06 '15 at 20:33
@HansPassant The comparer object itself could be thread-unsafe, for example if it caches the last-used strings. The documentation for `StringComparer` repeats the general claim "Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe." Having two threads that both use `StringComparer.OrdinalComparer.Equals` without some form of synchronisation is not guaranteed to work. (But I can admit that it's more likely that the docs get fixed to say this is supported, than that this will ever break.) – Jun 07 '15 at 00:12

StringComparer.CurrentCultureIgnoreCase efficiency with multiple calls in .NET

2 Answers2