Avoid lock contention in MemberInfoCache (#40116)
authorLevi Broderick <GrabYourPitchforks@users.noreply.github.com>
Sat, 1 Aug 2020 03:00:08 +0000 (20:00 -0700)
committerGitHub <noreply@github.com>
Sat, 1 Aug 2020 03:00:08 +0000 (20:00 -0700)
src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs

index a883bad..0dbfbf7 100644 (file)
@@ -234,6 +234,48 @@ namespace System
 
                 internal MethodBase AddMethod(RuntimeType declaringType, RuntimeMethodHandleInternal method, CacheType cacheType)
                 {
+                    // First, see if we've already cached an RuntimeMethodInfo or
+                    // RuntimeConstructorInfo that corresponds to this member. Since another
+                    // thread could be updating the backing store at the same time it's
+                    // possible that the check below will result in a false negative. That's
+                    // ok; we'll handle any concurrency issues in the later call to Insert.
+
+                    T?[]? allMembersLocal = m_allMembers;
+                    if (allMembersLocal != null)
+                    {
+                        // if not a Method or a Constructor, fall through
+                        if (cacheType == CacheType.Method)
+                        {
+                            foreach (T? candidate in allMembersLocal)
+                            {
+                                if (candidate is null)
+                                {
+                                    break; // end of list; stop iteration and fall through to slower path
+                                }
+
+                                if (candidate is RuntimeMethodInfo candidateRMI && candidateRMI.MethodHandle.Value == method.Value)
+                                {
+                                    return candidateRMI; // match!
+                                }
+                            }
+                        }
+                        else if (cacheType == CacheType.Constructor)
+                        {
+                            foreach (T? candidate in allMembersLocal)
+                            {
+                                if (candidate is null)
+                                {
+                                    break; // end of list; stop iteration and fall through to slower path
+                                }
+
+                                if (candidate is RuntimeConstructorInfo candidateRCI && candidateRCI.MethodHandle.Value == method.Value)
+                                {
+                                    return candidateRCI; // match!
+                                }
+                            }
+                        }
+                    }
+
                     T[] list = null!;
                     MethodAttributes methodAttributes = RuntimeMethodHandle.GetAttributes(method);
                     bool isPublic = (methodAttributes & MethodAttributes.MemberAccessMask) == MethodAttributes.Public;
@@ -264,6 +306,29 @@ namespace System
 
                 internal FieldInfo AddField(RuntimeFieldHandleInternal field)
                 {
+                    // First, see if we've already cached an RtFieldInfo that corresponds
+                    // to this field. Since another thread could be updating the backing
+                    // store at the same time it's possible that the check below will
+                    // result in a false negative. That's ok; we'll handle any concurrency
+                    // issues in the later call to Insert.
+
+                    T?[]? allMembersLocal = m_allMembers;
+                    if (allMembersLocal != null)
+                    {
+                        foreach (T? candidate in allMembersLocal)
+                        {
+                            if (candidate is null)
+                            {
+                                break; // end of list; stop iteration and fall through to slower path
+                            }
+
+                            if (candidate is RtFieldInfo candidateRtFI && candidateRtFI.GetFieldHandle() == field.Value)
+                            {
+                                return candidateRtFI; // match!
+                            }
+                        }
+                    }
+
                     // create the runtime field info
                     FieldAttributes fieldAttributes = RuntimeFieldHandle.GetAttributes(field);
                     bool isPublic = (fieldAttributes & FieldAttributes.FieldAccessMask) == FieldAttributes.Public;
@@ -507,7 +572,7 @@ namespace System
                             }
 
                             Debug.Assert(cachedMembers![freeSlotIndex] == null);
-                            cachedMembers[freeSlotIndex] = newMemberInfo;
+                            Volatile.Write(ref cachedMembers[freeSlotIndex], newMemberInfo); // value may be read outside of lock
                             freeSlotIndex++;
                         }
                     }