Merge pull request #9768 from VSadov/perf05

More performance improvements 

src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs

@@ -34,7 +34,7 @@ internal partial class Binder
             // Create a map from type parameter name to ordinal.
             // No need to report duplicate names since duplicates
             // are reported when the type parameters are bound.
-            var names = new Dictionary<string, int>(n);
+            var names = new Dictionary<string, int>(n, StringOrdinalComparer.Instance);
             foreach (var typeParameter in typeParameters)
             {
                 var name = typeParameter.Name;

src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
+using Microsoft.CodeAnalysis.Collections;
 
 /*
 SPEC:
@@ -2224,122 +2225,129 @@ private bool Fix(int iParam, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
                 return false;
             }
 
-            var candidates = new HashSet<TypeSymbol>();
+            var candidates = PooledHashSet<TypeSymbol>.GetInstance();
+            try
+            {
 
-            // Optimization: if we have one exact bound then we need not add any
-            // inexact bounds; we're just going to remove them anyway.
+                // Optimization: if we have one exact bound then we need not add any
+                // inexact bounds; we're just going to remove them anyway.
 
-            if (exact == null)
-            {
-                if (lower != null)
+                if (exact == null)
                 {
-                    foreach (var lowerBound in lower)
+                    if (lower != null)
                     {
-                        candidates.Add(lowerBound);
+                        foreach (var lowerBound in lower)
+                        {
+                            candidates.Add(lowerBound);
+                        }
                     }
-                }
-                if (upper != null)
-                {
-                    foreach (var upperBound in upper)
+                    if (upper != null)
                     {
-                        candidates.Add(upperBound);
+                        foreach (var upperBound in upper)
+                        {
+                            candidates.Add(upperBound);
+                        }
                     }
                 }
-            }
-            else
-            {
-                candidates.Add(exact.First());
-            }
+                else
+                {
+                    candidates.Add(exact.First());
+                }
 
-            if (candidates.Count == 0)
-            {
-                return false;
-            }
+                if (candidates.Count == 0)
+                {
+                    return false;
+                }
 
-            // Don't mutate the collection as we're iterating it.
-            var initialCandidates = candidates.ToList();
+                // Don't mutate the collection as we're iterating it.
+                var initialCandidates = candidates.ToList();
 
-            // SPEC:   For each lower bound U of Xi all types to which there is not an
-            // SPEC:   implicit conversion from U are removed from the candidate set.
+                // SPEC:   For each lower bound U of Xi all types to which there is not an
+                // SPEC:   implicit conversion from U are removed from the candidate set.
 
-            if (lower != null)
-            {
-                foreach (var bound in lower)
+                if (lower != null)
                 {
-                    // Make a copy; don't modify the collection as we're iterating it.
-                    foreach (var candidate in initialCandidates)
+                    foreach (var bound in lower)
                     {
-                        if (bound != candidate && !ImplicitConversionExists(bound, candidate, ref useSiteDiagnostics))
+                        // Make a copy; don't modify the collection as we're iterating it.
+                        foreach (var candidate in initialCandidates)
                         {
-                            candidates.Remove(candidate);
+                            if (bound != candidate && !ImplicitConversionExists(bound, candidate, ref useSiteDiagnostics))
+                            {
+                                candidates.Remove(candidate);
+                            }
                         }
                     }
                 }
-            }
 
-            // SPEC:   For each upper bound U of Xi all types from which there is not an
-            // SPEC:   implicit conversion to U are removed from the candidate set.
+                // SPEC:   For each upper bound U of Xi all types from which there is not an
+                // SPEC:   implicit conversion to U are removed from the candidate set.
 
-            if (upper != null)
-            {
-                foreach (var bound in upper)
+                if (upper != null)
                 {
-                    foreach (var candidate in initialCandidates)
+                    foreach (var bound in upper)
                     {
-                        if (bound != candidate && !ImplicitConversionExists(candidate, bound, ref useSiteDiagnostics))
+                        foreach (var candidate in initialCandidates)
                         {
-                            candidates.Remove(candidate);
+                            if (bound != candidate && !ImplicitConversionExists(candidate, bound, ref useSiteDiagnostics))
+                            {
+                                candidates.Remove(candidate);
+                            }
                         }
                     }
                 }
-            }
 
-            // SPEC: * If among the remaining candidate types there is a unique type V to
-            // SPEC:   which there is an implicit conversion from all the other candidate
-            // SPEC:   types, then the parameter is fixed to V.
+                // SPEC: * If among the remaining candidate types there is a unique type V to
+                // SPEC:   which there is an implicit conversion from all the other candidate
+                // SPEC:   types, then the parameter is fixed to V.
 
-            // SPEC: 4.7 The Dynamic Type
-            //       Type inference (7.5.2) will prefer dynamic over object if both are candidates.
-            // This rule doesn't have to be implemented explicitly due to special handling of 
-            // conversions from dynamic in ImplicitConversionExists helper.
+                // SPEC: 4.7 The Dynamic Type
+                //       Type inference (7.5.2) will prefer dynamic over object if both are candidates.
+                // This rule doesn't have to be implemented explicitly due to special handling of 
+                // conversions from dynamic in ImplicitConversionExists helper.
 
-            TypeSymbol best = null;
-            foreach (var candidate in candidates)
-            {
-                foreach (var candidate2 in candidates)
+                TypeSymbol best = null;
+                foreach (var candidate in candidates)
                 {
-                    if (candidate != candidate2 && !ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics))
+                    foreach (var candidate2 in candidates)
                     {
-                        goto OuterBreak;
+                        if (candidate != candidate2 && !ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics))
+                        {
+                            goto OuterBreak;
+                        }
+                    }
+
+                    if ((object)best == null)
+                    {
+                        best = candidate;
                     }
+                    else if (!(best.IsDynamic() && candidate.IsObjectType()))
+                    {
+                        Debug.Assert(!(best.IsObjectType() && candidate.IsDynamic()));
+                        Debug.Assert(!(best.IsDynamic() && candidate.IsObjectType()));
+
+                        // best candidate is not unique
+                        return false;
+                    }
+
+                    OuterBreak:
+                    ;
                 }
 
                 if ((object)best == null)
                 {
-                    best = candidate;
-                }
-                else if (!(best.IsDynamic() && candidate.IsObjectType()))
-                {
-                    Debug.Assert(!(best.IsObjectType() && candidate.IsDynamic()));
-                    Debug.Assert(!(best.IsDynamic() && candidate.IsObjectType()));
-
-                    // best candidate is not unique
+                    // no best candidate
                     return false;
                 }
 
-            OuterBreak:
-                ;
+                _fixedResults[iParam] = best;
+                UpdateDependenciesAfterFix(iParam);
+                return true;
             }
-
-            if ((object)best == null)
+            finally
             {
-                // no best candidate
-                return false;
+                candidates.Free();
             }
-
-            _fixedResults[iParam] = best;
-            UpdateDependenciesAfterFix(iParam);
-            return true;
         }
 
         private bool ImplicitConversionExists(TypeSymbol source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics)

src/Compilers/CSharp/Portable/Declarations/MergedNamespaceDeclaration.cs

@@ -2,6 +2,7 @@
 
 using System.Collections.Immutable;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
@@ -148,7 +149,7 @@ private ImmutableArray<MergedNamespaceOrTypeDeclaration> MakeChildren()
                 }
                 else
                 {
-                    var namespaceGroups = namespaces.ToDictionary(n => n.Name);
+                    var namespaceGroups = namespaces.ToDictionary(n => n.Name, StringOrdinalComparer.Instance);
                     namespaces.Free();
 
                     foreach (var namespaceGroup in namespaceGroups.Values)

src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs

@@ -16,8 +16,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Emit
 {
     internal sealed class CSharpSymbolMatcher : SymbolMatcher
     {
-        private static readonly StringComparer s_nameComparer = StringComparer.Ordinal;
-
         private readonly MatchDefs _defs;
         private readonly MatchSymbols _symbols;
 
@@ -113,7 +111,7 @@ private Cci.IDefinition VisitDefInternal(Cci.IDefinition def)
                         return null;
                     }
 
-                    return this.VisitTypeMembers(otherContainer, nestedType, GetNestedTypes, (a, b) => s_nameComparer.Equals(a.Name, b.Name));
+                    return this.VisitTypeMembers(otherContainer, nestedType, GetNestedTypes, (a, b) => StringOrdinalComparer.Equals(a.Name, b.Name));
                 }
 
                 var member = def as Cci.ITypeDefinitionMember;
@@ -128,7 +126,7 @@ private Cci.IDefinition VisitDefInternal(Cci.IDefinition def)
                     var field = def as Cci.IFieldDefinition;
                     if (field != null)
                     {
-                        return this.VisitTypeMembers(otherContainer, field, GetFields, (a, b) => s_nameComparer.Equals(a.Name, b.Name));
+                        return this.VisitTypeMembers(otherContainer, field, GetFields, (a, b) => StringOrdinalComparer.Equals(a.Name, b.Name));
                     }
                 }
 
@@ -161,7 +159,7 @@ private Cci.INamespaceTypeDefinition VisitNamespaceType(Cci.INamespaceTypeDefini
             {
                 if (_lazyTopLevelTypes == null)
                 {
-                    var typesByName = new Dictionary<string, Cci.INamespaceTypeDefinition>(s_nameComparer);
+                    var typesByName = new Dictionary<string, Cci.INamespaceTypeDefinition>(StringOrdinalComparer.Instance);
                     foreach (var type in this.GetTopLevelTypes())
                     {
                         // All generated top-level types are assumed to be in the global namespace.
@@ -667,19 +665,19 @@ private bool AreArrayTypesEqual(ArrayTypeSymbol type, ArrayTypeSymbol other)
 
             private bool AreEventsEqual(EventSymbol @event, EventSymbol other)
             {
-                Debug.Assert(s_nameComparer.Equals(@event.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(@event.Name, other.Name));
                 return _comparer.Equals(@event.Type, other.Type);
             }
 
             private bool AreFieldsEqual(FieldSymbol field, FieldSymbol other)
             {
-                Debug.Assert(s_nameComparer.Equals(field.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(field.Name, other.Name));
                 return _comparer.Equals(field.Type, other.Type);
             }
 
             private bool AreMethodsEqual(MethodSymbol method, MethodSymbol other)
             {
-                Debug.Assert(s_nameComparer.Equals(method.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(method.Name, other.Name));
 
                 Debug.Assert(method.IsDefinition);
                 Debug.Assert(other.IsDefinition);
@@ -708,7 +706,7 @@ private static MethodSymbol SubstituteTypeParameters(MethodSymbol method)
 
             private bool AreNamedTypesEqual(NamedTypeSymbol type, NamedTypeSymbol other)
             {
-                Debug.Assert(s_nameComparer.Equals(type.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(type.Name, other.Name));
                 // TODO: Test with overloads (from PE base class?) that have modifiers.
                 Debug.Assert(!type.HasTypeArgumentsCustomModifiers);
                 Debug.Assert(!other.HasTypeArgumentsCustomModifiers);
@@ -718,7 +716,7 @@ private bool AreNamedTypesEqual(NamedTypeSymbol type, NamedTypeSymbol other)
             private bool AreParametersEqual(ParameterSymbol parameter, ParameterSymbol other)
             {
                 Debug.Assert(parameter.Ordinal == other.Ordinal);
-                return s_nameComparer.Equals(parameter.Name, other.Name) &&
+                return StringOrdinalComparer.Equals(parameter.Name, other.Name) &&
                     (parameter.RefKind == other.RefKind) &&
                     _comparer.Equals(parameter.Type, other.Type);
             }
@@ -734,7 +732,7 @@ private bool ArePointerTypesEqual(PointerTypeSymbol type, PointerTypeSymbol othe
 
             private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other)
             {
-                Debug.Assert(s_nameComparer.Equals(property.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(property.Name, other.Name));
                 return _comparer.Equals(property.Type, other.Type) &&
                     property.Parameters.SequenceEqual(other.Parameters, AreParametersEqual);
             }
@@ -742,7 +740,7 @@ private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other)
             private bool AreTypeParametersEqual(TypeParameterSymbol type, TypeParameterSymbol other)
             {
                 Debug.Assert(type.Ordinal == other.Ordinal);
-                Debug.Assert(s_nameComparer.Equals(type.Name, other.Name));
+                Debug.Assert(StringOrdinalComparer.Equals(type.Name, other.Name));
                 // Comparing constraints is unnecessary: two methods cannot differ by
                 // constraints alone and changing the signature of a method is a rude
                 // edit. Furthermore, comparing constraint types might lead to a cycle.
@@ -796,7 +794,7 @@ private bool AreTypesEqual(TypeSymbol type, TypeSymbol other)
                     members.AddRange(synthesizedMembers);
                 }
 
-                var result = members.ToDictionary(s => ((Symbol)s).Name, s_nameComparer);
+                var result = members.ToDictionary(s => ((Symbol)s).Name, StringOrdinalComparer.Instance);
                 members.Free();
                 return result;
             }

src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs

@@ -826,8 +826,9 @@ string Cci.INamespaceTypeReference.NamespaceName
                 // INamespaceTypeReference is a type contained in a namespace
                 // if this method is called for a nested type, we are in big trouble.
                 Debug.Assert(((Cci.ITypeReference)this).AsNamespaceTypeReference != null);
-                return this.ContainingSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat);
-            }
+
+                return this.ContainingNamespace.QualifiedName;
+           }
         }
 
         bool Cci.INamespaceTypeDefinition.IsPublic

src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs

@@ -472,7 +472,7 @@ public override IEnumerable<Cci.ITypeReference> GetExportedTypes(EmitContext con
 
                 if (_lazyExportedTypes.Length > 0)
                 {
-                    var exportedNamesMap = new Dictionary<string, NamedTypeSymbol>();
+                    var exportedNamesMap = new Dictionary<string, NamedTypeSymbol>(StringOrdinalComparer.Instance);
 
                     // Report name collisions.
                     foreach (var exportedType in _lazyExportedTypes)

src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs

@@ -244,7 +244,7 @@ private XmlNodeSyntax ParseXmlElement()
                                 endName = this.WithXmlParseError(endName, XmlParseErrorCode.XML_InvalidWhitespace);
                             }
 
-                            if (!endName.IsMissing && name.ToString() != endName.ToString())
+                            if (!endName.IsMissing && !MatchingXmlNames(name, endName))
                             {
                                 endName = this.WithXmlParseError(endName, XmlParseErrorCode.XML_ElementTypeMatch, endName.ToString(), name.ToString());
                             }
@@ -288,6 +288,30 @@ private XmlNodeSyntax ParseXmlElement()
                 _pool.Free(attrs);
             }
         }
+
+        private static bool MatchingXmlNames(XmlNameSyntax name, XmlNameSyntax endName)
+        {
+            // PERF: because of deduplication we often get the same name for name and endName,
+            //       so we will check for such case first before materializing text for entire nodes 
+            //       and comparing that.
+            if (name == endName)
+            {
+                return true;
+            }
+
+            // before doing ToString, check if 
+            // all nodes contributing to ToString are recursively the same
+            // NOTE: leading and trailing trivia do not contribute to ToString
+            if (!name.HasLeadingTrivia && 
+                !endName.HasTrailingTrivia &&
+                name.IsEquivalentTo(endName))
+            {
+                return true;
+            }
+
+            return name.ToString() == endName.ToString();
+        }
+
         // assuming this is not used concurrently
         private readonly HashSet<string> _attributesSeen = new HashSet<string>();
 

src/Compilers/CSharp/Portable/Parser/QuickScanner.cs

@@ -28,8 +28,11 @@ private enum QuickScanState : byte
             Dot,
             CompoundPunctStart,
             DoneAfterNext,
+            // we are relying on Bad state immediately following Done 
+            // to be able to detect exiting conditions in one "state >= Done" test.
+            // And we are also relying on this to be the last item in the enum.
             Done,
-            Bad
+            Bad = Done + 1    
         }
 
         private enum CharFlags : byte
@@ -209,7 +212,13 @@ private SyntaxToken QuickScanSyntaxToken()
                 var flags = uc < charPropLength ? (CharFlags)s_charProperties[uc] : CharFlags.Complex;
 
                 state = (QuickScanState)s_stateTransitions[(int)state, (int)flags];
-                if (state == QuickScanState.Done || state == QuickScanState.Bad)
+                // NOTE: that Bad > Done and it is the only state like that
+                // as a result, we will exit the loop on either Bad or Done.
+                // the assert below will validate that these are the only states on which we exit
+                // Also note that we must exit on Done or Bad
+                // since the state machine does not have transitions for these states 
+                // and will promptly fail if we do not exit.
+                if (state >= QuickScanState.Done)
                 {
                     goto exitWhile;
                 }
@@ -221,7 +230,7 @@ private SyntaxToken QuickScanSyntaxToken()
         exitWhile:
 
             TextWindow.AdvanceChar(i - TextWindow.Offset);
-            Debug.Assert(state == QuickScanState.Bad || state == QuickScanState.Done);
+            Debug.Assert(state == QuickScanState.Bad || state == QuickScanState.Done, "can only exit with Bad or Done");
 
             if (state == QuickScanState.Done)
             {

src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
+using Microsoft.CodeAnalysis.Collections;
 
 namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
@@ -496,16 +497,18 @@ private static bool HasDuplicateInterfaces(NamedTypeSymbol type, ConsList<Symbol
                     return false;
 
                 default:
-                    var set = new HashSet<NamedTypeSymbol>(ReferenceEqualityComparer.Instance);
+                    var set = PooledHashSet<object>.GetInstance();
                     foreach (var i in array)
                     {
                         if (!set.Add(i.OriginalDefinition))
                         {
+                            set.Free();
                             goto hasRelatedInterfaces;
                         }
                     }
 
                     // all interfaces are unrelated
+                    set.Free();
                     return false;
             }
             

src/Compilers/CSharp/Portable/Symbols/LabelSymbol.cs

@@ -13,24 +13,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
     /// </summary>
     internal abstract class LabelSymbol : Symbol, ILabelSymbol
     {
-        internal LabelSymbol(string name)
-        {
-            _name = name;
-        }
-
-        private readonly string _name;
-
-        /// <summary>
-        /// Gets the name of this label
-        /// </summary>
-        public override string Name
-        {
-            get
-            {
-                return _name;
-            }
-        }
-
         /// <summary>
         /// Returns false because label can't be defined externally.
         /// </summary>

src/Compilers/CSharp/Portable/Symbols/MergedNamespaceSymbol.cs

@@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
     /// namespaces. Any sub-namespaces with the same names are also merged if they have two or more
     /// instances.
     /// 
-    /// Merged namespaces are used to merged the symbols from multiple metadata modules and the
+    /// Merged namespaces are used to merge the symbols from multiple metadata modules and the
     /// source "module" into a single symbol tree that represents all the available symbols. The
     /// compiler resolves names against this merged set of symbols.
     /// 

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs

@@ -22,7 +22,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE
     /// </summary>
     internal abstract class PENamedTypeSymbol : NamedTypeSymbol
     {
-        private static readonly Dictionary<string, ImmutableArray<PENamedTypeSymbol>> s_emptyNestedTypes = new Dictionary<string, ImmutableArray<PENamedTypeSymbol>>();
+        private static readonly Dictionary<string, ImmutableArray<PENamedTypeSymbol>> s_emptyNestedTypes = new Dictionary<string, ImmutableArray<PENamedTypeSymbol>>(EmptyComparer.Instance);
 
         private readonly NamespaceOrTypeSymbol _container;
         private readonly TypeDefinitionHandle _handle;
@@ -1585,10 +1585,10 @@ public override TypeKind TypeKind
         {
             get
             {
-                if (_lazyKind == TypeKind.Unknown)
-                {
-                    TypeKind result;
+                TypeKind result = _lazyKind;
 
+                if (result == TypeKind.Unknown)
+                {
                     if (_flags.IsInterface())
                     {
                         result = TypeKind.Interface;
@@ -1603,22 +1603,26 @@ public override TypeKind TypeKind
                         {
                             SpecialType baseCorTypeId = @base.SpecialType;
 
-                            // Code is cloned from MetaImport::DoImportBaseAndImplements()
-                            if (baseCorTypeId == SpecialType.System_Enum)
-                            {
-                                // Enum
-                                result = TypeKind.Enum;
-                            }
-                            else if (baseCorTypeId == SpecialType.System_MulticastDelegate)
+                            switch (baseCorTypeId)
                             {
-                                // Delegate
-                                result = TypeKind.Delegate;
-                            }
-                            else if (baseCorTypeId == SpecialType.System_ValueType &&
-                                     this.SpecialType != SpecialType.System_Enum)
-                            {
-                                // Struct
-                                result = TypeKind.Struct;
+                                case SpecialType.System_Enum:
+                                    // Enum
+                                    result = TypeKind.Enum;
+                                    break;
+
+                                case SpecialType.System_MulticastDelegate:
+                                    // Delegate
+                                    result = TypeKind.Delegate;
+                                    break;
+
+                                case SpecialType.System_ValueType:
+                                    // System.Enum is the only class that derives from ValueType
+                                    if (this.SpecialType != SpecialType.System_Enum)
+                                    {
+                                        // Struct
+                                        result = TypeKind.Struct;
+                                    }
+                                    break;
                             }
                         }
                     }
@@ -1626,7 +1630,7 @@ public override TypeKind TypeKind
                     _lazyKind = result;
                 }
 
-                return _lazyKind;
+                return result;
             }
         }
 
@@ -1873,7 +1877,7 @@ private PEMethodSymbol GetAccessorMethod(PEModule module, Dictionary<MethodDefin
 
         private static Dictionary<string, ImmutableArray<Symbol>> GroupByName(ArrayBuilder<Symbol> symbols)
         {
-            return symbols.ToDictionary(s => s.Name);
+            return symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance);
         }
 
         private static Dictionary<string, ImmutableArray<PENamedTypeSymbol>> GroupByName(ArrayBuilder<PENamedTypeSymbol> symbols)
@@ -1883,7 +1887,7 @@ private PEMethodSymbol GetAccessorMethod(PEModule module, Dictionary<MethodDefin
                 return s_emptyNestedTypes;
             }
 
-            return symbols.ToDictionary(s => s.Name);
+            return symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance);
         }
 
 

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamespaceSymbol.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using Roslyn.Utilities;
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
@@ -180,7 +181,7 @@ protected void LoadAllMembers(IEnumerable<IGrouping<string, TypeDefinitionHandle
 
             MetadataHelpers.GetInfoForImmediateNamespaceMembers(
                 isGlobalNamespace,
-                isGlobalNamespace ? 0 : this.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat).Length,
+                isGlobalNamespace ? 0 : GetQualifiedNameLength(),
                 typesByNS,
                 StringComparer.Ordinal,
                 out nestedTypes, out nestedNamespaces);
@@ -190,6 +191,21 @@ protected void LoadAllMembers(IEnumerable<IGrouping<string, TypeDefinitionHandle
             LazyInitializeTypes(nestedTypes);
         }
 
+        private int GetQualifiedNameLength()
+        {
+            int length = this.Name.Length;
+
+            var parent = ContainingNamespace;
+            while (parent?.IsGlobalNamespace == false)
+            {
+                // add name of the parent + "."
+                length += parent.Name.Length + 1;
+                parent = parent.ContainingNamespace;
+            }
+
+            return length;
+        }
+
         /// <summary>
         /// Create symbols for nested namespaces and initialize namespaces map.
         /// </summary>
@@ -201,7 +217,7 @@ protected void LoadAllMembers(IEnumerable<IGrouping<string, TypeDefinitionHandle
                 var children = from child in childNamespaces
                                select new PENestedNamespaceSymbol(child.Key, this, child.Value);
 
-                var namespaces = new Dictionary<string, PENestedNamespaceSymbol>();
+                var namespaces = new Dictionary<string, PENestedNamespaceSymbol>(StringOrdinalComparer.Instance);
 
                 foreach (var c in children)
                 {
@@ -241,7 +257,7 @@ private void LazyInitializeTypes(IEnumerable<IGrouping<string, TypeDefinitionHan
 
                                 if (noPiaLocalTypes == null)
                                 {
-                                    noPiaLocalTypes = new Dictionary<string, TypeDefinitionHandle>();
+                                    noPiaLocalTypes = new Dictionary<string, TypeDefinitionHandle>(StringOrdinalComparer.Instance);
                                 }
 
                                 noPiaLocalTypes[typeDefName] = t;
@@ -252,7 +268,7 @@ private void LazyInitializeTypes(IEnumerable<IGrouping<string, TypeDefinitionHan
                     }
                 }
 
-                var typesDict = children.ToDictionary(c => c.Name);
+                var typesDict = children.ToDictionary(c => c.Name, StringOrdinalComparer.Instance);
                 children.Free();
 
                 if (noPiaLocalTypes != null)

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

@@ -364,21 +364,26 @@ internal void GetExtensionMethods(ArrayBuilder<MethodSymbol> methods, string nam
         {
             if (this.MightContainExtensionMethods)
             {
-                var members = nameOpt == null
-                    ? this.GetMembersUnordered()
-                    : this.GetSimpleNonTypeMembers(nameOpt);
+                DoGetExtensionMethods(methods, nameOpt, arity, options);
+            }
+        }
 
-                foreach (var member in members)
+        internal void DoGetExtensionMethods(ArrayBuilder<MethodSymbol> methods, string nameOpt, int arity, LookupOptions options)
+        {
+            var members = nameOpt == null
+                ? this.GetMembersUnordered()
+                : this.GetSimpleNonTypeMembers(nameOpt);
+
+            foreach (var member in members)
+            {
+                if (member.Kind == SymbolKind.Method)
                 {
-                    if (member.Kind == SymbolKind.Method)
+                    var method = (MethodSymbol)member;
+                    if (method.IsExtensionMethod &&
+                        ((options & LookupOptions.AllMethodsOnArityZero) != 0 || arity == method.Arity))
                     {
-                        var method = (MethodSymbol)member;
-                        if (method.IsExtensionMethod &&
-                            ((options & LookupOptions.AllMethodsOnArityZero) != 0 || arity == method.Arity))
-                        {
-                            Debug.Assert(method.MethodKind != MethodKind.ReducedExtension);
-                            methods.Add(method);
-                        }
+                        Debug.Assert(method.MethodKind != MethodKind.ReducedExtension);
+                        methods.Add(method);
                     }
                 }
             }

src/Compilers/CSharp/Portable/Symbols/NamespaceSymbol.cs

@@ -12,6 +12,10 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
     /// </summary>
     internal abstract partial class NamespaceSymbol : NamespaceOrTypeSymbol, INamespaceSymbol
     {
+        // PERF: initialization of the following fields will allocate, so we make them lazy
+        private ImmutableArray<NamedTypeSymbol> _lazyTypesMightContainExtensionMethods;
+        private string _lazyQualifiedName;
+        
         // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
         // Changes to the public interface of this class should remain synchronized with the VB version.
         // Do not make any changes to the public interface without making the corresponding change
@@ -311,6 +315,22 @@ internal NamespaceSymbol GetNestedNamespace(NameSyntax name)
             return null;
         }
 
+        private ImmutableArray<NamedTypeSymbol> TypesMightContainExtensionMethods
+        {
+            get
+            {
+                var typesWithExtensionMethods = this._lazyTypesMightContainExtensionMethods;
+                if (typesWithExtensionMethods.IsDefault)
+                {
+                    this._lazyTypesMightContainExtensionMethods = this.GetTypeMembersUnordered().WhereAsArray(t => t.MightContainExtensionMethods);
+                    typesWithExtensionMethods = this._lazyTypesMightContainExtensionMethods;
+                }
+
+                return typesWithExtensionMethods;
+            }
+        }
+
+
         /// <summary>
         /// Add all extension methods in this namespace to the given list. If name or arity
         /// or both are provided, only those extension methods that match are included.
@@ -332,10 +352,11 @@ internal virtual void GetExtensionMethods(ArrayBuilder<MethodSymbol> methods, st
                 return;
             }
 
-            var types = this.GetTypeMembersUnordered();
-            foreach (var type in types)
+            var typesWithExtensionMethods = this.TypesMightContainExtensionMethods;
+
+            foreach (var type in typesWithExtensionMethods)
             {
-                type.GetExtensionMethods(methods, nameOpt, arity, options);
+                type.DoGetExtensionMethods(methods, nameOpt, arity, options);
             }
         }
 
@@ -377,6 +398,15 @@ ImmutableArray<INamespaceSymbol> INamespaceSymbol.ConstituentNamespaces
             }
         }
 
+        internal string QualifiedName
+        {
+            get
+            {
+                return _lazyQualifiedName ??
+                    (_lazyQualifiedName = this.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat));
+            }
+        }
+
         #endregion
 
         #region ISymbol Members

src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs

@@ -2521,7 +2521,7 @@ internal override NamedTypeSymbol TryLookupForwardedMetadataTypeWithCycleDetecti
 
                 if (wellKnownAttributeData != null && wellKnownAttributeData.ForwardedTypes != null)
                 {
-                    forwardedTypesFromSource = new Dictionary<string, NamedTypeSymbol>();
+                    forwardedTypesFromSource = new Dictionary<string, NamedTypeSymbol>(StringOrdinalComparer.Instance);
 
                     foreach (NamedTypeSymbol forwardedType in wellKnownAttributeData.ForwardedTypes)
                     {

src/Compilers/CSharp/Portable/Symbols/Source/SourceLabelSymbol.cs

@@ -17,21 +17,48 @@ internal sealed class SourceLabelSymbol : LabelSymbol
         /// </summary>
         private readonly ConstantValue _switchCaseLabelConstant;
 
+        // PERF: Often we do not need this, so we make this lazy
+        private string _lazyName;
+
         public SourceLabelSymbol(
             MethodSymbol containingMethod,
             SyntaxNodeOrToken identifierNodeOrToken,
             ConstantValue switchCaseLabelConstant = null)
-            : base(identifierNodeOrToken.IsToken ? identifierNodeOrToken.AsToken().ValueText : identifierNodeOrToken.ToString())
         {
             _containingMethod = containingMethod;
             _identifierNodeOrToken = identifierNodeOrToken;
             _switchCaseLabelConstant = switchCaseLabelConstant;
         }
 
+        public override string Name
+        {
+            get
+            {
+                return _lazyName ??
+                    (_lazyName = MakeLabelName());
+            }
+        }
+
+        private string MakeLabelName()
+        {
+            var node = _identifierNodeOrToken.AsNode();
+            if (node != null)
+            {
+                return node.ToString();
+            }
+
+            var tk = _identifierNodeOrToken.AsToken();
+            if (tk.Kind() != SyntaxKind.None)
+            {
+                return tk.ValueText;
+            }
+
+            return _switchCaseLabelConstant?.ToString() ?? "";
+        }
+
         public SourceLabelSymbol(
             MethodSymbol containingMethod,
-            ConstantValue switchCaseLabelConstant = null)
-            : base(switchCaseLabelConstant.ToString())
+            ConstantValue switchCaseLabelConstant)
         {
             _containingMethod = containingMethod;
             _identifierNodeOrToken = default(SyntaxToken);

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Runtime.InteropServices;
 using System.Threading;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -154,7 +155,7 @@ public void SetIsManagedType(bool isManagedType)
         private Dictionary<string, ImmutableArray<Symbol>> _lazyMembersDictionary;
         private Dictionary<string, ImmutableArray<Symbol>> _lazyEarlyAttributeDecodingMembersDictionary;
 
-        private static readonly Dictionary<string, ImmutableArray<NamedTypeSymbol>> s_emptyTypeMembers = new Dictionary<string, ImmutableArray<NamedTypeSymbol>>();
+        private static readonly Dictionary<string, ImmutableArray<NamedTypeSymbol>> s_emptyTypeMembers = new Dictionary<string, ImmutableArray<NamedTypeSymbol>>(EmptyComparer.Instance);
         private Dictionary<string, ImmutableArray<NamedTypeSymbol>> _lazyTypeMembers;
         private ImmutableArray<Symbol> _lazyMembersFlattened;
         private ImmutableArray<SynthesizedExplicitImplementationForwardingMethod> _lazySynthesizedExplicitImplementations;
@@ -1080,7 +1081,9 @@ public override ImmutableArray<NamedTypeSymbol> GetTypeMembers(string name, int
                 }
 
                 Debug.Assert(s_emptyTypeMembers.Count == 0);
-                return symbols.Count > 0 ? symbols.ToDictionary(s => s.Name) : s_emptyTypeMembers;
+                return symbols.Count > 0 ? 
+                    symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance) : 
+                    s_emptyTypeMembers;
             }
             finally
             {
@@ -1227,12 +1230,7 @@ internal override ImmutableArray<Symbol> GetEarlyAttributeDecodingMembers(string
                 // NOTE: members were added in a single pass over the syntax, so they're already
                 // in lexical order.
 
-                // TODO: Can we move ToDictionary() off ArrayBuilder<T> so that we don't need a temp here?
-                var temp = ArrayBuilder<Symbol>.GetInstance();
-                temp.AddRange(membersAndInitializers.NonTypeNonIndexerMembers);
-                var membersByName = temp.ToDictionary(s => s.Name);
-                temp.Free();
-
+                var membersByName = membersAndInitializers.NonTypeNonIndexerMembers.ToDictionary(s => s.Name);
                 AddNestedTypesToDictionary(membersByName, GetTypeMembersDictionary());
 
                 Interlocked.CompareExchange(ref _lazyEarlyAttributeDecodingMembersDictionary, membersByName, null);
@@ -2168,7 +2166,7 @@ private static bool ContainsModifier(SyntaxTokenList modifiers, SyntaxKind modif
                 merged.Add(indexerMembers[indexerPos]);
             }
 
-            var membersByName = merged.ToDictionary(s => s.Name);
+            var membersByName = merged.ToDictionary(s => s.Name, StringOrdinalComparer.Instance);
             merged.Free();
 
             return membersByName;

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamespaceSymbol.cs

@@ -248,7 +248,7 @@ internal override NamespaceExtent Extent
                 // NOTE: This method depends on MakeNameToMembersMap() on creating a proper 
                 // NOTE: type of the array, see comments in MakeNameToMembersMap() for details
 
-                var dictionary = new Dictionary<String, ImmutableArray<NamedTypeSymbol>>();
+                var dictionary = new Dictionary<String, ImmutableArray<NamedTypeSymbol>>(StringOrdinalComparer.Instance);
 
                 Dictionary<String, ImmutableArray<NamespaceOrTypeSymbol>> map = this.GetNameToMembersMap();
                 foreach (var kvp in map)
@@ -484,7 +484,7 @@ private struct NameToSymbolMapBuilder
 
             public NameToSymbolMapBuilder(int capacity)
             {
-                _dictionary = new Dictionary<string, object>(capacity);
+                _dictionary = new Dictionary<string, object>(capacity, StringOrdinalComparer.Instance);
             }
 
             public void Add(NamespaceOrTypeSymbol symbol)
@@ -510,7 +510,7 @@ public void Add(NamespaceOrTypeSymbol symbol)
 
             public Dictionary<String, ImmutableArray<NamespaceOrTypeSymbol>> CreateMap()
             {
-                var result = new Dictionary<String, ImmutableArray<NamespaceOrTypeSymbol>>(_dictionary.Count);
+                var result = new Dictionary<String, ImmutableArray<NamespaceOrTypeSymbol>>(_dictionary.Count, StringOrdinalComparer.Instance);
 
                 foreach (var kvp in _dictionary)
                 {

src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedLabelSymbol.cs

@@ -9,9 +9,19 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
     internal sealed class GeneratedLabelSymbol : LabelSymbol
     {
+        private readonly string _name;
+
         public GeneratedLabelSymbol(string name)
-            : base(LabelName(name))
         {
+            _name = LabelName(name);
+        }
+
+        public override string Name
+        {
+            get
+            {
+                return _name;
+            }
         }
 
 #if DEBUG

src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs

@@ -68,13 +68,13 @@ internal new SyntaxTree SyntaxTree
         {
             get
             {
-                ComputeSyntaxTree(this);
-                Debug.Assert(this._syntaxTree != null);
-                return this._syntaxTree;
+                var result =  this._syntaxTree ?? ComputeSyntaxTree(this);
+                Debug.Assert(result != null);
+                return result;
             }
         }
 
-        private static void ComputeSyntaxTree(CSharpSyntaxNode node)
+        private static SyntaxTree ComputeSyntaxTree(CSharpSyntaxNode node)
         {
             ArrayBuilder<CSharpSyntaxNode> nodes = null;
             SyntaxTree tree = null;
@@ -91,21 +91,21 @@ private static void ComputeSyntaxTree(CSharpSyntaxNode node)
                 var parent = node.Parent;
                 if (parent == null)
                 {
+                    // set the tree on the root node atomically
                     Interlocked.CompareExchange(ref node._syntaxTree, CSharpSyntaxTree.CreateWithoutClone(node), null);
                     tree = node._syntaxTree;
                     break;
                 }
-                else if (parent._syntaxTree != null)
+
+                tree = parent._syntaxTree;
+                if (tree != null)
                 {
-                    Interlocked.CompareExchange(ref node._syntaxTree, parent._syntaxTree, null);
-                    tree = node._syntaxTree;
+                    node._syntaxTree = tree;
                     break;
                 }
-                else
-                {
-                    (nodes ?? (nodes = ArrayBuilder<CSharpSyntaxNode>.GetInstance())).Add(node);
-                    node = parent;
-                }
+
+                (nodes ?? (nodes = ArrayBuilder<CSharpSyntaxNode>.GetInstance())).Add(node);
+                node = parent;
             }
 
             // Propagate the syntax tree downwards if necessary
@@ -115,15 +115,21 @@ private static void ComputeSyntaxTree(CSharpSyntaxNode node)
 
                 foreach (var n in nodes)
                 {
-                    var existingTree = Interlocked.CompareExchange(ref n._syntaxTree, tree, null);
+                    var existingTree =  n._syntaxTree;
                     if (existingTree != null)
                     {
-                        tree = existingTree;
+                        Debug.Assert(existingTree == tree, "how could this node belong to a different tree?");
+
+                        // yield the race
+                        break;
                     }
+                    n._syntaxTree = tree;
                 }
 
                 nodes.Free();
             }
+
+            return tree;
         }
 
         public abstract TResult Accept<TResult>(CSharpSyntaxVisitor<TResult> visitor);
@@ -262,7 +268,7 @@ public override void WriteTo(System.IO.TextWriter writer)
             this.Green.WriteTo(writer, true, true);
         }
 
-        #region serialization
+#region serialization
 
 
         private static readonly RecordingObjectBinder s_defaultBinder = new ConcurrentRecordingObjectBinder();
@@ -393,7 +399,7 @@ private static IEnumerable<object> GetSerializationData()
 
             return s_serializationData;
         }
-        #endregion
+#endregion
 
         /// <summary>
         /// Determines whether this node is structurally equivalent to another.
@@ -451,7 +457,7 @@ internal override SyntaxNode GetLambda()
             return LambdaUtilities.GetLambda(this);
         }
 
-        #region Directives
+#region Directives
 
         internal IList<DirectiveTriviaSyntax> GetDirectives(Func<DirectiveTriviaSyntax, bool> filter = null)
         {
@@ -538,9 +544,9 @@ public DirectiveTriviaSyntax GetLastDirective(Func<DirectiveTriviaSyntax, bool>
             return null;
         }
 
-        #endregion
+#endregion
 
-        #region Node Lookup
+#region Node Lookup
 
         /// <summary>
         /// Returns child node or token that contains given position.
@@ -558,9 +564,9 @@ public override SyntaxNodeOrToken ChildThatContainsPosition(int position)
             Debug.Assert(childNodeOrToken.FullSpan.Contains(position), "ChildThatContainsPosition's return value does not contain the requested position.");
             return childNodeOrToken;
         }
-        #endregion
+#endregion
 
-        #region Token Lookup
+#region Token Lookup
 
         /// <summary>
         /// Gets the first token of the tree rooted by this node.
@@ -664,9 +670,9 @@ internal SyntaxToken FindTokenIncludingCrefAndNameAttributes(int position)
             return nonTriviaToken;
         }
 
-        #endregion
+#endregion
 
-        #region Trivia Lookup
+#region Trivia Lookup
 
         /// <summary>
         /// Finds a descendant trivia of this node at the specified position, where the position is
@@ -693,9 +699,9 @@ public new SyntaxTrivia FindTrivia(int position, bool findInsideTrivia = false)
             return base.FindTrivia(position, findInsideTrivia);
         }
 
-        #endregion
+#endregion
 
-        #region SyntaxNode members
+#region SyntaxNode members
 
         /// <summary>
         /// Determine if this node is structurally equivalent to another.
@@ -771,6 +777,6 @@ protected override bool IsEquivalentToCore(SyntaxNode node, bool topLevel = fals
             return SyntaxFactory.AreEquivalent(this, (CSharpSyntaxNode)node, topLevel);
         }
 
-        #endregion
+#endregion
     }
 }

src/Compilers/CSharp/Test/Syntax/LexicalAndXml/XmlDocCommentTests.cs

@@ -2862,6 +2862,40 @@ public class Program
                 Diagnostic(ErrorCode.WRN_XMLParseError, " "));
         }
 
+        [Fact]
+        public void WhitespaceInXmlEndName()
+        {
+            var text = @"
+/// <A:B>
+///   good
+/// </A:B>
+/// <A:B>
+///   bad
+/// </A :B>
+/// <A:B>
+///   bad
+/// </A: B>
+public class Program
+{
+}";
+
+            var tree = Parse(text);
+            tree.GetDiagnostics().Verify(
+                // (7,7): warning CS1570: XML comment has badly formed XML -- 'End tag 'A :B' does not match the start tag 'A:B'.'
+                // /// </A :B>
+                Diagnostic(ErrorCode.WRN_XMLParseError, "A :B").WithArguments("A :B", "A:B").WithLocation(7, 7),
+                // (7,8): warning CS1570: XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'
+                // /// </A :B>
+                Diagnostic(ErrorCode.WRN_XMLParseError, " ").WithLocation(7, 8),
+                // (10,7): warning CS1570: XML comment has badly formed XML -- 'End tag 'A: B' does not match the start tag 'A:B'.'
+                // /// </A: B>
+                Diagnostic(ErrorCode.WRN_XMLParseError, "A: B").WithArguments("A: B", "A:B").WithLocation(10, 7),
+                // (10,9): warning CS1570: XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'
+                // /// </A: B>
+                Diagnostic(ErrorCode.WRN_XMLParseError, " ").WithLocation(10, 9)
+                );
+        }
+
         [Fact]
         [Trait("Feature", "Xml Documentation Comments")]
         public void TestDocumentationComment()

src/Compilers/Core/Portable/CodeAnalysis.csproj

@@ -62,6 +62,8 @@
     <Compile Include="Binding\AbstractLookupSymbolsInfo.cs" />
     <Compile Include="CaseInsensitiveComparison.cs" />
     <Compile Include="CodeGen\ILEmitStyle.cs" />
+    <Compile Include="InternalUtilities\EmptyComparer.cs" />
+    <Compile Include="InternalUtilities\StringOrdinalComparer.cs" />
     <Compile Include="MetadataReference\AssemblyIdentityMap.cs" />
     <Compile Include="Compilation\OperationVisitor.cs" />
     <Compile Include="Compilation\OperationWalker.cs" />

src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs

@@ -456,5 +456,49 @@ public static int Count<T>(this ImmutableArray<T> items, Func<T, bool> predicate
 
             return count;
         }
+
+        internal static Dictionary<K, ImmutableArray<T>> ToDictionary<K, T>(this ImmutableArray<T> items, Func<T, K> keySelector, IEqualityComparer<K> comparer = null)
+        {
+            if (items.Length == 1)
+            {
+                var dictionary1 = new Dictionary<K, ImmutableArray<T>>(1, comparer);
+                T value = items[0];
+                dictionary1.Add(keySelector(value), ImmutableArray.Create(value));
+                return dictionary1;
+            }
+
+            if (items.Length == 0)
+            {
+                return new Dictionary<K, ImmutableArray<T>>(comparer);
+            }
+
+            // bucketize
+            // prevent reallocation. it may not have 'count' entries, but it won't have more. 
+            var accumulator = new Dictionary<K, ArrayBuilder<T>>(items.Length, comparer);
+            for (int i = 0; i < items.Length; i++)
+            {
+                var item = items[i];
+                var key = keySelector(item);
+
+                ArrayBuilder<T> bucket;
+                if (!accumulator.TryGetValue(key, out bucket))
+                {
+                    bucket = ArrayBuilder<T>.GetInstance();
+                    accumulator.Add(key, bucket);
+                }
+
+                bucket.Add(item);
+            }
+
+            var dictionary = new Dictionary<K, ImmutableArray<T>>(accumulator.Count, comparer);
+
+            // freeze
+            foreach (var pair in accumulator)
+            {
+                dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree());
+            }
+
+            return dictionary;
+        }
     }
 }

src/Compilers/Core/Portable/Diagnostic/NoLocation.cs

@@ -21,7 +21,7 @@ public override LocationKind Kind
 
         public override bool Equals(object obj)
         {
-            return ReferenceEquals(this, obj);
+            return (object)this == obj;
         }
 
         public override int GetHashCode()

src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalyzer.cs

@@ -23,12 +23,12 @@ public abstract class DiagnosticAnalyzer
 
         public sealed override bool Equals(object obj)
         {
-            return ReferenceEqualityComparer.Instance.Equals(this, obj);
+            return (object)this == obj;
         }
 
         public sealed override int GetHashCode()
         {
-            return ReferenceEqualityComparer.Instance.GetHashCode(this);
+            return ReferenceEqualityComparer.GetHashCode(this);
         }
 
         public sealed override string ToString()

src/Compilers/Core/Portable/DocumentationComments/DocumentationProvider.NullDocumentationProvider.cs

@@ -22,7 +22,7 @@ protected internal override string GetDocumentationForSymbol(string documentatio
             public override bool Equals(object obj)
             {
                 // Only one instance is expected to exist, so reference equality is fine.
-                return ReferenceEquals(this, obj);
+                return (object)this == obj;
             }
 
             public override int GetHashCode()

src/Compilers/Core/Portable/InternalUtilities/EmptyComparer.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+
+namespace Roslyn.Utilities
+{
+    /// <summary>
+    /// Very cheap trivial comparer that never matches the keys,
+    /// should only be used in empty dictionaries.
+    /// </summary>
+    internal sealed class EmptyComparer : IEqualityComparer<object>
+    {
+        public static readonly EmptyComparer Instance = new EmptyComparer();
+
+        private EmptyComparer()
+        {
+        }
+
+        bool IEqualityComparer<object>.Equals(object a, object b)
+        {
+            Debug.Assert(false, "Are we using empty comparer with nonempty dictionary?");
+            return false;
+        }
+
+        int IEqualityComparer<object>.GetHashCode(object s)
+        {
+            // dictionary will call this often
+            return 0;
+        }
+    }
+}

src/Compilers/Core/Portable/InternalUtilities/ReferenceEqualityComparer.cs

@@ -10,7 +10,7 @@ namespace Roslyn.Utilities
     /// </summary>
     internal class ReferenceEqualityComparer : IEqualityComparer<object>
     {
-        public static readonly IEqualityComparer<object> Instance = new ReferenceEqualityComparer();
+        public static readonly ReferenceEqualityComparer Instance = new ReferenceEqualityComparer();
 
         private ReferenceEqualityComparer()
         {
@@ -22,6 +22,11 @@ bool IEqualityComparer<object>.Equals(object a, object b)
         }
 
         int IEqualityComparer<object>.GetHashCode(object a)
+        {
+            return ReferenceEqualityComparer.GetHashCode(a);
+        }
+
+        public static int GetHashCode(object a)
         {
             return RuntimeHelpers.GetHashCode(a);
         }

src/Compilers/Core/Portable/InternalUtilities/StringOrdinalComparer.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+
+namespace Roslyn.Utilities
+{
+    /// <summary>
+    /// Compares string based upon their ordinal equality.
+    /// We use this comparer for string identifiers because it does exactly what we need and nothing more
+    /// The StringComparer.Ordinal as implemented by StringComparer is more complex to support 
+    /// case sensitive and insensitive compares depending on flags.
+    /// It also defers to the default string hash function that might not be the best for our scenarios.
+    /// </summary>
+    internal sealed class StringOrdinalComparer : IEqualityComparer<string>
+    {
+        public static readonly StringOrdinalComparer Instance = new StringOrdinalComparer();
+
+        private StringOrdinalComparer()
+        {
+        }
+
+        bool IEqualityComparer<string>.Equals(string a, string b)
+        {
+            return StringOrdinalComparer.Equals(a, b);
+        }
+
+        public static bool Equals(string a, string b)
+        {
+            // this is fast enough
+            return string.Equals(a, b);
+        }
+
+        int IEqualityComparer<string>.GetHashCode(string s)
+        {
+            // PERF: the default string hashcode is not always good or fast and cannot be changed for compat reasons.
+            // We, however, can use anything we want in our dictionaries. 
+            // Our typical scenario is a relatively short string (identifier)
+            // FNV performs pretty well in such cases
+            return Hash.GetFNVHashCode(s);
+        }
+    }
+}

src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs

@@ -99,23 +99,25 @@ internal TypeSymbol GetTypeOfToken(EntityHandle token, out bool isNoPiaLocalType
             TypeSymbol type;
             HandleKind tokenType = token.Kind;
 
-            if (tokenType == HandleKind.TypeDefinition)
+            switch (tokenType)
             {
-                type = GetTypeOfTypeDef((TypeDefinitionHandle)token, out isNoPiaLocalType, isContainingType: false);
-            }
-            else if (tokenType == HandleKind.TypeSpecification)
-            {
-                isNoPiaLocalType = false;
-                type = GetTypeOfTypeSpec((TypeSpecificationHandle)token);
-            }
-            else if (tokenType == HandleKind.TypeReference)
-            {
-                type = GetTypeOfTypeRef((TypeReferenceHandle)token, out isNoPiaLocalType);
-            }
-            else
-            {
-                isNoPiaLocalType = false;
-                type = GetUnsupportedMetadataTypeSymbol();
+                case HandleKind.TypeDefinition:
+                    type = GetTypeOfTypeDef((TypeDefinitionHandle)token, out isNoPiaLocalType, isContainingType: false);
+                    break;
+
+                case HandleKind.TypeSpecification:
+                    isNoPiaLocalType = false;
+                    type = GetTypeOfTypeSpec((TypeSpecificationHandle)token);
+                    break;
+
+                case HandleKind.TypeReference:
+                    type = GetTypeOfTypeRef((TypeReferenceHandle)token, out isNoPiaLocalType);
+                    break;
+
+                default:
+                    isNoPiaLocalType = false;
+                    type = GetUnsupportedMetadataTypeSymbol();
+                    break;
             }
 
             Debug.Assert(type != null);

src/Compilers/Core/Portable/Syntax/SyntaxNode.cs

@@ -141,8 +141,8 @@ internal SyntaxNode GetRed(ref SyntaxNode field, int slot)
                 var green = this.Green.GetSlot(slot);
                 if (green != null)
                 {
-                    result = green.CreateRed(this, this.GetChildPosition(slot));
-                    result = Interlocked.CompareExchange(ref field, result, null) ?? result;
+                    Interlocked.CompareExchange(ref field, green.CreateRed(this, this.GetChildPosition(slot)), null);
+                    result = field;
                 }
             }
 
@@ -159,8 +159,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field)
                 var green = this.Green.GetSlot(0);
                 if (green != null)
                 {
-                    result = green.CreateRed(this, this.Position);
-                    result = Interlocked.CompareExchange(ref field, result, null) ?? result;
+                    Interlocked.CompareExchange(ref field, green.CreateRed(this, this.Position), null);
+                    result = field;
                 }
             }
 
@@ -176,8 +176,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field)
                 var green = this.Green.GetSlot(slot);
                 if (green != null)
                 {
-                    result = (T)green.CreateRed(this, this.GetChildPosition(slot));
-                    result = Interlocked.CompareExchange(ref field, result, null) ?? result;
+                    Interlocked.CompareExchange(ref field, (T)green.CreateRed(this, this.GetChildPosition(slot)), null);
+                    result = field;
                 }
             }
 
@@ -194,8 +194,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field)
                 var green = this.Green.GetSlot(0);
                 if (green != null)
                 {
-                    result = (T)green.CreateRed(this, this.Position);
-                    result = Interlocked.CompareExchange(ref field, result, null) ?? result;
+                    Interlocked.CompareExchange(ref field, (T)green.CreateRed(this, this.Position), null);
+                    result = field;
                 }
             }
 
@@ -216,11 +216,9 @@ internal SyntaxNode GetRedElement(ref SyntaxNode element, int slot)
             if (result == null)
             {
                 var green = this.Green.GetSlot(slot);
-                result = green.CreateRed(this.Parent, this.GetChildPosition(slot)); // <- passing list's parent
-                if (Interlocked.CompareExchange(ref element, result, null) != null)
-                {
-                    result = element;
-                }
+                // passing list's parent
+                Interlocked.CompareExchange(ref element, green.CreateRed(this.Parent, this.GetChildPosition(slot)), null);
+                result = element;
             }
 
             return result;
@@ -240,11 +238,9 @@ internal SyntaxNode GetRedElementIfNotToken(ref SyntaxNode element)
                 var green = this.Green.GetSlot(1);
                 if (!green.IsToken)
                 {
-                    result = green.CreateRed(this.Parent, this.GetChildPosition(1)); // <- passing list's parent
-                    if (Interlocked.CompareExchange(ref element, result, null) != null)
-                    {
-                        result = element;
-                    }
+                    // passing list's parent
+                    Interlocked.CompareExchange(ref element, green.CreateRed(this.Parent, this.GetChildPosition(1)), null);
+                    result = element;
                 }
             }
 

src/Compilers/Core/SharedCollections/ArrayBuilder.cs

@@ -352,41 +352,44 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
         {
             if (this.Count == 1)
             {
-                var dictionary = new Dictionary<K, ImmutableArray<T>>(1, comparer);
+                var dictionary1 = new Dictionary<K, ImmutableArray<T>>(1, comparer);
                 T value = this[0];
-                dictionary.Add(keySelector(value), ImmutableArray.Create(value));
-                return dictionary;
+                dictionary1.Add(keySelector(value), ImmutableArray.Create(value));
+                return dictionary1;
             }
-            else
+
+            if (this.Count == 0)
             {
-                // bucketize
-                // prevent reallocation. it may not have 'count' entries, but it won't have more. 
-                var accumulator = new Dictionary<K, ArrayBuilder<T>>(Count, comparer);
-                for (int i = 0; i < Count; i++)
-                {
-                    var item = this[i];
-                    var key = keySelector(item);
+                return new Dictionary<K, ImmutableArray<T>>(comparer);
+            }
 
-                    ArrayBuilder<T> bucket;
-                    if (!accumulator.TryGetValue(key, out bucket))
-                    {
-                        bucket = ArrayBuilder<T>.GetInstance();
-                        accumulator.Add(key, bucket);
-                    }
+            // bucketize
+            // prevent reallocation. it may not have 'count' entries, but it won't have more. 
+            var accumulator = new Dictionary<K, ArrayBuilder<T>>(Count, comparer);
+            for (int i = 0; i < Count; i++)
+            {
+                var item = this[i];
+                var key = keySelector(item);
 
-                    bucket.Add(item);
+                ArrayBuilder<T> bucket;
+                if (!accumulator.TryGetValue(key, out bucket))
+                {
+                    bucket = ArrayBuilder<T>.GetInstance();
+                    accumulator.Add(key, bucket);
                 }
 
-                var dictionary = new Dictionary<K, ImmutableArray<T>>(accumulator.Count, comparer);
+                bucket.Add(item);
+            }
 
-                // freeze
-                foreach (var pair in accumulator)
-                {
-                    dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree());
-                }
+            var dictionary = new Dictionary<K, ImmutableArray<T>>(accumulator.Count, comparer);
 
-                return dictionary;
+            // freeze
+            foreach (var pair in accumulator)
+            {
+                dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree());
             }
+
+            return dictionary;
         }
 
         public void AddRange(ArrayBuilder<T> items)

src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs

@@ -51,7 +51,7 @@ protected override string GetDocumentationForSymbol(string documentationMemberID
 
             public override bool Equals(object obj)
             {
-                return ReferenceEquals(this, obj);
+                return (object)this == obj;
             }
 
             public override int GetHashCode()