Avoid allocating Regex when using static methods (#496)
authorStephen Toub <stoub@microsoft.com>
Wed, 4 Dec 2019 22:02:08 +0000 (17:02 -0500)
committerGitHub <noreply@github.com>
Wed, 4 Dec 2019 22:02:08 +0000 (17:02 -0500)
* Avoid allocating Regex when using static methods

Regex supports a cache that's meant to be used for the static methods, e.g. the static Regex.IsMatch.  However, currently that cache isn't caching the actual Regex class, but rather a sub object that stores some of the state from the Regex and then is used to rehydrate a new Regex instance.  This means we allocate a new Regex object when these static methods are used, even if the data is found in the cache.  This means an operation like the static Regex.IsMatch is never allocation-free.  There's also weirdness around this caching, in that when a new Regex instance is constructed, it checks the cache (paying the relevant lookup costs) even though it doesn't add back to the cache, so the only way it would ever find something in the cache is if the same set of inputs (e.g. pattern, options, timeout, etc.) are used with both the Regex ctor and with the static methods, where the static methods would populate the cache that's then consulted by the ctor.  This adds unnecessary expense on the common path for a very uncommon situation; it also complicates the code non-trivially.

This commit changes things to cleanly separate the cache from Regex, and to actually cache the Regex instance itself.

* Address PR feedback

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Cache.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Match.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Replace.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Split.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Timeout.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs
src/libraries/System.Text.RegularExpressions/tests/Regex.Cache.Tests.cs

index adf4eff..9469f3c 100644 (file)
@@ -2,46 +2,53 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using SysDebug = System.Diagnostics.Debug;  // as Regex.Debug
 using System.Collections.Generic;
-using System.Collections;
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
 using System.Runtime.CompilerServices;
 using System.Threading;
-using System.Diagnostics.CodeAnalysis;
-using System.Runtime.InteropServices;
 
 namespace System.Text.RegularExpressions
 {
     public partial class Regex
     {
+        public static int CacheSize
+        {
+            get => RegexCache.CacheSize;
+            set => RegexCache.CacheSize = value;
+        }
+    }
+
+    internal sealed class RegexCache
+    {
+        private const int DefaultMaxCacheSize = 15;
         private const int CacheDictionarySwitchLimit = 10;
 
-        private static int s_cacheSize = 15;
-        // the cache of code and factories that are currently loaded:
-        // Dictionary for large cache
-        private static readonly Dictionary<CachedCodeEntryKey, CachedCodeEntry> s_cache = new Dictionary<CachedCodeEntryKey, CachedCodeEntry>(s_cacheSize);
-        // linked list for LRU and for small cache
+        private static readonly Dictionary<Key, Node> s_cache = new Dictionary<Key, Node>(DefaultMaxCacheSize);
+        private static Node? s_cacheFirst, s_cacheLast; // linked list for LRU and for small cache
+        private static int s_maxCacheSize = DefaultMaxCacheSize;
         private static int s_cacheCount = 0;
-        private static CachedCodeEntry? s_cacheFirst;
-        private static CachedCodeEntry? s_cacheLast;
 
         public static int CacheSize
         {
-            get => s_cacheSize;
+            get => s_maxCacheSize;
             set
             {
                 if (value < 0)
+                {
                     throw new ArgumentOutOfRangeException(nameof(value));
+                }
 
                 lock (s_cache)
                 {
-                    s_cacheSize = value;  // not to allow other thread to change it while we use cache
-                    while (s_cacheCount > s_cacheSize)
+                    s_maxCacheSize = value;  // not to allow other thread to change it while we use cache
+                    while (s_cacheCount > s_maxCacheSize)
                     {
-                        CachedCodeEntry last = s_cacheLast!;
+                        Node last = s_cacheLast!;
                         if (s_cacheCount >= CacheDictionarySwitchLimit)
                         {
-                            SysDebug.Assert(s_cache.ContainsKey(last.Key));
+                            Debug.Assert(s_cache.ContainsKey(last.Key));
                             s_cache.Remove(last.Key);
                         }
 
@@ -49,14 +56,14 @@ namespace System.Text.RegularExpressions
                         s_cacheLast = last.Next;
                         if (last.Next != null)
                         {
-                            SysDebug.Assert(s_cacheFirst != null);
-                            SysDebug.Assert(s_cacheFirst != last);
-                            SysDebug.Assert(last.Next.Previous == last);
+                            Debug.Assert(s_cacheFirst != null);
+                            Debug.Assert(s_cacheFirst != last);
+                            Debug.Assert(last.Next.Previous == last);
                             last.Next.Previous = null;
                         }
                         else // last one removed
                         {
-                            SysDebug.Assert(s_cacheFirst == last);
+                            Debug.Assert(s_cacheFirst == last);
                             s_cacheFirst = null;
                         }
 
@@ -66,123 +73,161 @@ namespace System.Text.RegularExpressions
             }
         }
 
-        /// <summary>
-        ///  Find cache based on options+pattern+culture and optionally add new cache if not found
-        /// </summary>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private CachedCodeEntry? GetCachedCode(CachedCodeEntryKey key, bool isToAdd)
+        public static Regex GetOrAdd(string pattern)
         {
-            // to avoid lock:
-            CachedCodeEntry? first = s_cacheFirst;
-            if (first != null && first.Key.Equals(key))
-                return first;
+            Regex.ValidatePattern(pattern);
 
-            if (s_cacheSize == 0)
-                return null;
+            CultureInfo culture = CultureInfo.CurrentCulture;
+            Key key = new Key(pattern, culture.ToString(), RegexOptions.None, hasTimeout: false);
 
-            return GetCachedCodeEntryInternal(key, isToAdd);
+            if (!TryGet(key, out Regex? regex))
+            {
+                regex = new Regex(pattern, culture);
+                Add(key, regex);
+            }
+
+            return regex;
         }
 
-        private CachedCodeEntry? GetCachedCodeEntryInternal(CachedCodeEntryKey key, bool isToAdd)
+        public static Regex GetOrAdd(string pattern, RegexOptions options, TimeSpan matchTimeout)
         {
-            lock (s_cache)
+            Regex.ValidatePattern(pattern);
+            Regex.ValidateOptions(options);
+            Regex.ValidateMatchTimeout(matchTimeout);
+
+            CultureInfo culture = (options & RegexOptions.CultureInvariant) != 0 ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture;
+            Key key = new Key(pattern, culture.ToString(), options, matchTimeout != Regex.InfiniteMatchTimeout);
+
+            if (!TryGet(key, out Regex? regex))
             {
-                // first look for it in the cache and move it to the head
-                CachedCodeEntry? entry = LookupCachedAndPromote(key);
+                regex = new Regex(pattern, options, matchTimeout, culture);
+                Add(key, regex);
+            }
 
-                // it wasn't in the cache, so we'll add a new one
-                if (entry == null && isToAdd && s_cacheSize != 0) // check cache size again in case it changed
+            return regex;
+        }
+
+        private static bool TryGet(Key key, [NotNullWhen(true)] out Regex? regex)
+        {
+            Node? cachedRegex = s_cacheFirst;
+            if (cachedRegex != null)
+            {
+                if (cachedRegex.Key.Equals(key))
                 {
-                    entry = new CachedCodeEntry(key, capnames!, capslist!, _code!, caps!, capsize, _runnerref!, _replref!);
+                    regex = cachedRegex.Regex;
+                    return true;
+                }
 
-                    // put first in linked list:
-                    if (s_cacheFirst != null)
+                if (s_maxCacheSize != 0)
+                {
+                    lock (s_cache)
                     {
-                        SysDebug.Assert(s_cacheFirst.Next == null);
-                        s_cacheFirst.Next = entry;
-                        entry.Previous = s_cacheFirst;
+                        cachedRegex = LookupCachedAndPromote(key);
+                        if (cachedRegex != null)
+                        {
+                            regex = cachedRegex.Regex;
+                            return true;
+                        }
                     }
-                    s_cacheFirst = entry;
+                }
+            }
 
-                    s_cacheCount++;
-                    if (s_cacheCount >= CacheDictionarySwitchLimit)
+            regex = null;
+            return false;
+        }
+
+        private static void Add(Key key, Regex regex)
+        {
+            lock (s_cache)
+            {
+                // If we're not supposed to cache, or if the entry is already cached, we're done.
+                if (s_maxCacheSize == 0 || LookupCachedAndPromote(key) != null)
+                {
+                    return;
+                }
+
+                // Create the entry for caching.
+                var entry = new Node(key, regex);
+
+                // Put it at the beginning of the linked list, as it is the most-recently used.
+                if (s_cacheFirst != null)
+                {
+                    Debug.Assert(s_cacheFirst.Next == null);
+                    s_cacheFirst.Next = entry;
+                    entry.Previous = s_cacheFirst;
+                }
+                s_cacheFirst = entry;
+                s_cacheCount++;
+
+                // If we've graduated to using the dictionary for lookups, add it to the dictionary.
+                if (s_cacheCount >= CacheDictionarySwitchLimit)
+                {
+                    if (s_cacheCount == CacheDictionarySwitchLimit)
                     {
-                        if (s_cacheCount == CacheDictionarySwitchLimit)
-                        {
-                            FillCacheDictionary();
-                        }
-                        else
+                        // If we just hit the threshold, we need to populate the dictionary from the list.
+                        s_cache.Clear();
+                        for (Node? next = s_cacheFirst; next != null; next = next.Previous)
                         {
-                            s_cache.Add(key, entry);
+                            s_cache.Add(next.Key, next);
                         }
-
-                        SysDebug.Assert(s_cacheCount == s_cache.Count);
                     }
-
-                    // update last in linked list:
-                    if (s_cacheLast == null)
+                    else
                     {
-                        s_cacheLast = entry;
+                        // If we've already populated the dictionary, just add this one entry.
+                        s_cache.Add(key, entry);
                     }
-                    else if (s_cacheCount > s_cacheSize) // remove last
-                    {
-                        CachedCodeEntry last = s_cacheLast;
-                        if (s_cacheCount >= CacheDictionarySwitchLimit)
-                        {
-                            SysDebug.Assert(s_cache[last.Key] == s_cacheLast);
-                            s_cache.Remove(last.Key);
-                        }
 
-                        SysDebug.Assert(last.Previous == null);
-                        SysDebug.Assert(last.Next != null);
-                        SysDebug.Assert(last.Next.Previous == last);
-                        last.Next.Previous = null;
-                        s_cacheLast = last.Next;
-                        s_cacheCount--;
-                    }
+                    Debug.Assert(s_cacheCount == s_cache.Count);
                 }
 
-                return entry;
-            }
-        }
+                // Update the tail of the linked list.  If nothing was cached, just set the tail.
+                // If we're over our cache limit, remove the tail.
+                if (s_cacheLast == null)
+                {
+                    s_cacheLast = entry;
+                }
+                else if (s_cacheCount > s_maxCacheSize)
+                {
+                    Node last = s_cacheLast;
+                    if (s_cacheCount >= CacheDictionarySwitchLimit)
+                    {
+                        Debug.Assert(s_cache[last.Key] == s_cacheLast);
+                        s_cache.Remove(last.Key);
+                    }
 
-        private void FillCacheDictionary()
-        {
-            s_cache.Clear();
-            CachedCodeEntry? next = s_cacheFirst;
-            while (next != null)
-            {
-                s_cache.Add(next.Key, next);
-                next = next.Previous;
+                    Debug.Assert(last.Previous == null);
+                    Debug.Assert(last.Next != null);
+                    Debug.Assert(last.Next.Previous == last);
+
+                    last.Next.Previous = null;
+                    s_cacheLast = last.Next;
+
+                    s_cacheCount--;
+                }
             }
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)] // Unprofitable inline - JIT overly pessimistic
-        private static bool TryGetCacheValue(CachedCodeEntryKey key, [NotNullWhen(true)] out CachedCodeEntry? entry)
+        [MethodImpl(MethodImplOptions.AggressiveInlining)] // single call site, separated out for convenience
+        private static bool TryGetCacheValueAfterFirst(Key key, [NotNullWhen(true)] out Node? entry)
         {
+            Debug.Assert(Monitor.IsEntered(s_cache));
+            Debug.Assert(s_cacheFirst != null);
+            Debug.Assert(s_cacheLast != null);
+
             if (s_cacheCount >= CacheDictionarySwitchLimit)
             {
-                SysDebug.Assert((s_cacheFirst != null && s_cacheLast != null && s_cache.Count > 0) ||
-                                (s_cacheFirst == null && s_cacheLast == null && s_cache.Count == 0),
-                                "Linked list and Dict should be synchronized");
+                Debug.Assert(s_cache.Count == s_cacheCount);
                 return s_cache.TryGetValue(key, out entry);
             }
 
-            return TryGetCacheValueSmall(key, out entry);
-        }
-
-        private static bool TryGetCacheValueSmall(CachedCodeEntryKey key, [NotNullWhen(true)] out CachedCodeEntry? entry)
-        {
-            CachedCodeEntry? current = s_cacheFirst; // first already checked
-            if (current != null)
+            for (Node? current = s_cacheFirst.Previous; // s_cacheFirst already checked by caller, so skip it here
+                 current != null;
+                 current = current.Previous)
             {
-                for (current = current.Previous; current != null; current = current.Previous)
+                if (current.Key.Equals(key))
                 {
-                    if (current.Key.Equals(key))
-                    {
-                        entry = current;
-                        return true;
-                    }
+                    entry = current;
+                    return true;
                 }
             }
 
@@ -190,31 +235,33 @@ namespace System.Text.RegularExpressions
             return false;
         }
 
-        private static CachedCodeEntry? LookupCachedAndPromote(CachedCodeEntryKey key)
+        private static Node? LookupCachedAndPromote(Key key)
         {
-            SysDebug.Assert(Monitor.IsEntered(s_cache));
+            Debug.Assert(Monitor.IsEntered(s_cache));
 
-            CachedCodeEntry? entry = s_cacheFirst;
+            Node? entry = s_cacheFirst;
             if (entry != null &&
                 !entry.Key.Equals(key) && // again check this as could have been promoted by other thread
-                TryGetCacheValue(key, out entry))
+                TryGetCacheValueAfterFirst(key, out entry))
             {
-                // promote:
-                SysDebug.Assert(s_cacheFirst != entry, "key should not get s_livecode_first");
-                SysDebug.Assert(s_cacheFirst != null, "as Dict has at least one");
-                SysDebug.Assert(s_cacheFirst.Next == null);
-                SysDebug.Assert(s_cacheFirst.Previous != null);
-                SysDebug.Assert(entry.Next != null, "not first so Next should exist");
-                SysDebug.Assert(entry.Next.Previous == entry);
+                // We found the item and it wasn't the first; it needs to be promoted.
+
+                Debug.Assert(s_cacheFirst != entry, "key should not get s_livecode_first");
+                Debug.Assert(s_cacheFirst != null, "as Dict has at least one");
+                Debug.Assert(s_cacheFirst.Next == null);
+                Debug.Assert(s_cacheFirst.Previous != null);
+                Debug.Assert(entry.Next != null, "not first so Next should exist");
+                Debug.Assert(entry.Next.Previous == entry);
+
                 if (s_cacheLast == entry)
                 {
-                    SysDebug.Assert(entry.Previous == null, "last");
+                    Debug.Assert(entry.Previous == null, "last");
                     s_cacheLast = entry.Next;
                 }
                 else
                 {
-                    SysDebug.Assert(entry.Previous != null, "in middle");
-                    SysDebug.Assert(entry.Previous.Next == entry);
+                    Debug.Assert(entry.Previous != null, "in middle");
+                    Debug.Assert(entry.Previous.Next == entry);
                     entry.Previous.Next = entry.Next;
                 }
                 entry.Next.Previous = entry.Previous;
@@ -228,88 +275,60 @@ namespace System.Text.RegularExpressions
             return entry;
         }
 
-        /// <summary>
-        /// Used as a key for CacheCodeEntry
-        /// </summary>
-        internal readonly struct CachedCodeEntryKey : IEquatable<CachedCodeEntryKey>
+        /// <summary>Used as a key for <see cref="Node"/>.</summary>
+        internal readonly struct Key : IEquatable<Key>
         {
             private readonly string _pattern;
-            private readonly string _cultureKey;
+            private readonly string _culture;
             private readonly RegexOptions _options;
             private readonly bool _hasTimeout;
 
-            public CachedCodeEntryKey(string pattern, string cultureKey, RegexOptions options, bool hasTimeout)
+            public Key(string pattern, string culture, RegexOptions options, bool hasTimeout)
             {
-                SysDebug.Assert(pattern != null, "Pattern must be provided");
-                SysDebug.Assert(cultureKey != null, "Culture must be provided");
+                Debug.Assert(pattern != null, "Pattern must be provided");
+                Debug.Assert(culture != null, "Culture must be provided");
 
                 _pattern = pattern;
-                _cultureKey = cultureKey;
+                _culture = culture;
                 _options = options;
                 _hasTimeout = hasTimeout;
             }
 
             public override bool Equals(object? obj) =>
-                obj is CachedCodeEntryKey other && Equals(other);
+                obj is Key other && Equals(other);
 
-            public bool Equals(CachedCodeEntryKey other) =>
+            public bool Equals(Key other) =>
                 _pattern.Equals(other._pattern) &&
-                _cultureKey.Equals(other._cultureKey) &&
+                _culture.Equals(other._culture) &&
                 _options == other._options &&
                 _hasTimeout == other._hasTimeout;
 
-            public static bool operator ==(CachedCodeEntryKey left, CachedCodeEntryKey right) =>
+            public static bool operator ==(Key left, Key right) =>
                 left.Equals(right);
 
-            public static bool operator !=(CachedCodeEntryKey left, CachedCodeEntryKey right) =>
+            public static bool operator !=(Key left, Key right) =>
                 !left.Equals(right);
 
             public override int GetHashCode() =>
                 _pattern.GetHashCode() ^
-                _cultureKey.GetHashCode() ^
+                _culture.GetHashCode() ^
                 ((int)_options);
                 // no need to include timeout in the hashcode; it'll almost always be the same
         }
 
-        /// <summary>
-        /// Used to cache byte codes
-        /// </summary>
-        internal sealed class CachedCodeEntry
+        /// <summary>Used to cache Regex instances.</summary>
+        private sealed class Node
         {
-            public CachedCodeEntry? Next;
-            public CachedCodeEntry? Previous;
-            public readonly CachedCodeEntryKey Key;
-            public RegexCode? Code;
-            public readonly Hashtable Caps;
-            public readonly Hashtable Capnames;
-            public readonly string[] Capslist;
-#if FEATURE_COMPILED
-            public RegexRunnerFactory? Factory;
-#endif
-            public readonly int Capsize;
-            public readonly ExclusiveReference Runnerref;
-            public readonly WeakReference<RegexReplacement?> ReplRef;
-
-            public CachedCodeEntry(CachedCodeEntryKey key, Hashtable capnames, string[] capslist, RegexCode code,
-                Hashtable caps, int capsize, ExclusiveReference runner, WeakReference<RegexReplacement?> replref)
-            {
-                Key = key;
-                Capnames = capnames;
-                Capslist = capslist;
-                Code = code;
-                Caps = caps;
-                Capsize = capsize;
-                Runnerref = runner;
-                ReplRef = replref;
-            }
+            public readonly Key Key;
+            public readonly Regex Regex;
+            public Node? Next;
+            public Node? Previous;
 
-#if FEATURE_COMPILED
-            public void AddCompiled(RegexRunnerFactory factory)
+            public Node(Key key, Regex regex)
             {
-                Factory = factory;
-                Code = null;
+                Key = key;
+                Regex = regex;
             }
-#endif
         }
     }
 }
index ea4dfb1..bba51ad 100644 (file)
@@ -10,7 +10,7 @@ namespace System.Text.RegularExpressions
         /// Searches the input string for one or more occurrences of the text supplied in the given pattern.
         /// </summary>
         public static bool IsMatch(string input, string pattern) =>
-            new Regex(pattern, addToCache: true).IsMatch(input);
+            RegexCache.GetOrAdd(pattern).IsMatch(input);
 
         /// <summary>
         /// Searches the input string for one or more occurrences of the text
@@ -18,10 +18,10 @@ namespace System.Text.RegularExpressions
         /// parameter.
         /// </summary>
         public static bool IsMatch(string input, string pattern, RegexOptions options) =>
-            IsMatch(input, pattern, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).IsMatch(input);
 
         public static bool IsMatch(string input, string pattern, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, addToCache: true).IsMatch(input);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).IsMatch(input);
 
         /*
          * Returns true if the regex finds a match within the specified string
@@ -59,7 +59,7 @@ namespace System.Text.RegularExpressions
         /// supplied in the pattern parameter.
         /// </summary>
         public static Match Match(string input, string pattern) =>
-            new Regex(pattern, addToCache: true).Match(input);
+            RegexCache.GetOrAdd(pattern).Match(input);
 
         /// <summary>
         /// Searches the input string for one or more occurrences of the text
@@ -67,10 +67,10 @@ namespace System.Text.RegularExpressions
         /// string.
         /// </summary>
         public static Match Match(string input, string pattern, RegexOptions options) =>
-            Match(input, pattern, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).Match(input);
 
         public static Match Match(string input, string pattern, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, addToCache: true).Match(input);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).Match(input);
 
         /*
          * Finds the first match for the regular expression starting at the beginning
@@ -123,16 +123,16 @@ namespace System.Text.RegularExpressions
         /// Returns all the successful matches as if Match were called iteratively numerous times.
         /// </summary>
         public static MatchCollection Matches(string input, string pattern) =>
-            new Regex(pattern, addToCache: true).Matches(input);
+            RegexCache.GetOrAdd(pattern).Matches(input);
 
         /// <summary>
         /// Returns all the successful matches as if Match were called iteratively numerous times.
         /// </summary>
         public static MatchCollection Matches(string input, string pattern, RegexOptions options) =>
-            Matches(input, pattern, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).Matches(input);
 
         public static MatchCollection Matches(string input, string pattern, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, addToCache: true).Matches(input);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).Matches(input);
 
         /*
          * Finds the first match for the regular expression starting at the beginning
index a213908..5a8c16c 100644 (file)
@@ -20,7 +20,7 @@ namespace System.Text.RegularExpressions
         /// the first character in the input string.
         /// </summary>
         public static string Replace(string input, string pattern, string replacement) =>
-            new Regex(pattern, addToCache: true).Replace(input, replacement);
+            RegexCache.GetOrAdd(pattern).Replace(input, replacement);
 
         /// <summary>
         /// Replaces all occurrences of
@@ -28,10 +28,10 @@ namespace System.Text.RegularExpressions
         /// pattern, starting at the first character in the input string.
         /// </summary>
         public static string Replace(string input, string pattern, string replacement, RegexOptions options) =>
-            Replace(input, pattern, replacement, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).Replace(input, replacement);
 
         public static string Replace(string input, string pattern, string replacement, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, true).Replace(input, replacement);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).Replace(input, replacement);
 
         /// <summary>
         /// Replaces all occurrences of the previously defined pattern with the
@@ -83,17 +83,17 @@ namespace System.Text.RegularExpressions
         /// replacement pattern.
         /// </summary>
         public static string Replace(string input, string pattern, MatchEvaluator evaluator) =>
-            new Regex(pattern, addToCache: true).Replace(input, evaluator);
+            RegexCache.GetOrAdd(pattern).Replace(input, evaluator);
 
         /// <summary>
         /// Replaces all occurrences of the <paramref name="pattern"/> with the recent
         /// replacement pattern, starting at the first character.
         /// </summary>
         public static string Replace(string input, string pattern, MatchEvaluator evaluator, RegexOptions options) =>
-            Replace(input, pattern, evaluator, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).Replace(input, evaluator);
 
         public static string Replace(string input, string pattern, MatchEvaluator evaluator, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, addToCache: true).Replace(input, evaluator);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).Replace(input, evaluator);
 
         /// <summary>
         /// Replaces all occurrences of the previously defined pattern with the recent
index aab119d..d0bb0e9 100644 (file)
@@ -13,16 +13,16 @@ namespace System.Text.RegularExpressions
         /// by <paramref name="pattern"/>.
         /// </summary>
         public static string[] Split(string input, string pattern) =>
-            new Regex(pattern, addToCache: true).Split(input);
+            RegexCache.GetOrAdd(pattern).Split(input);
 
         /// <summary>
         /// Splits the <paramref name="input "/>string at the position defined by <paramref name="pattern"/>.
         /// </summary>
         public static string[] Split(string input, string pattern, RegexOptions options) =>
-            Split(input, pattern, options, s_defaultMatchTimeout);
+            RegexCache.GetOrAdd(pattern, options, s_defaultMatchTimeout).Split(input);
 
         public static string[] Split(string input, string pattern, RegexOptions options, TimeSpan matchTimeout) =>
-            new Regex(pattern, options, matchTimeout, addToCache: true).Split(input);
+            RegexCache.GetOrAdd(pattern, options, matchTimeout).Split(input);
 
         /// <summary>
         /// Splits the <paramref name="input"/> string at the position defined by a
index 0d9bd1a..f82231b 100644 (file)
@@ -36,26 +36,6 @@ namespace System.Text.RegularExpressions
         /// </summary>
         public TimeSpan MatchTimeout => internalMatchTimeout;
 
-        // Note: "&lt;" is the XML entity for smaller ("<").
-        /// <summary>
-        /// Validates that the specified match timeout value is valid.
-        /// The valid range is <code>TimeSpan.Zero &lt; matchTimeout &lt;= Regex.MaximumMatchTimeout</code>.
-        /// </summary>
-        /// <param name="matchTimeout">The timeout value to validate.</param>
-        /// <exception cref="ArgumentOutOfRangeException">If the specified timeout is not within a valid range.
-        /// </exception>
-        protected internal static void ValidateMatchTimeout(TimeSpan matchTimeout)
-        {
-            if (InfiniteMatchTimeout == matchTimeout)
-                return;
-
-            // Change this to make sure timeout is not longer then Environment.Ticks cycle length:
-            if (TimeSpan.Zero < matchTimeout && matchTimeout <= s_maximumMatchTimeout)
-                return;
-
-            throw new ArgumentOutOfRangeException(nameof(matchTimeout));
-        }
-
         /// <summary>
         /// Specifies the default RegEx matching timeout value (i.e. the timeout that will be used if no
         /// explicit timeout is specified).
index 0d9b0e2..f828a37 100644 (file)
@@ -2,9 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// The Regex class represents a single compiled instance of a regular
-// expression.
-
 using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
@@ -14,18 +11,16 @@ using System.Reflection.Emit;
 using System.Runtime.CompilerServices;
 #endif
 using System.Runtime.Serialization;
-using System.Threading;
 
 namespace System.Text.RegularExpressions
 {
     /// <summary>
-    /// Represents an immutable, compiled regular expression. Also
-    /// contains static methods that allow use of regular expressions without instantiating
-    /// a Regex explicitly.
+    /// Represents an immutable regular expression. Also contains static methods that
+    /// allow use of regular expressions without instantiating a Regex explicitly.
     /// </summary>
     public partial class Regex : ISerializable
     {
-        internal const int MaxOptionShift = 10;
+        private const int MaxOptionShift = 10;
 
         protected internal string? pattern;                   // The string pattern provided
         protected internal RegexOptions roptions;             // the top-level options from the options string
@@ -35,10 +30,10 @@ namespace System.Text.RegularExpressions
         protected internal string[]? capslist;                // if captures are sparse or named captures are used, this is the sorted list of names
         protected internal int capsize;                       // the size of the capture array
 
-        internal ExclusiveReference? _runnerref;              // cached runner
         internal WeakReference<RegexReplacement?>? _replref;  // cached parsed replacement pattern
-        internal RegexCode? _code;                            // if interpreted, this is the code for RegexInterpreter
-        internal bool _refsInitialized = false;
+        private ExclusiveReference? _runnerref;               // cached runner
+        private RegexCode? _code;                             // if interpreted, this is the code for RegexInterpreter
+        private bool _refsInitialized = false;
 
         protected Regex()
         {
@@ -46,136 +41,122 @@ namespace System.Text.RegularExpressions
         }
 
         /// <summary>
-        /// Creates and compiles a regular expression object for the specified regular
-        /// expression.
+        /// Creates a regular expression object for the specified regular expression.
         /// </summary>
-        public Regex(string pattern) =>
-            Init(pattern, RegexOptions.None, s_defaultMatchTimeout, addToCache: false);
+        public Regex(string pattern) :
+            this(pattern, culture: null)
+        {
+        }
 
         /// <summary>
-        /// Creates and compiles a regular expression object for the specified regular
-        /// expression, and adds it to the cache.
+        /// Creates a regular expression object for the specified regular expression, with options that modify the pattern.
         /// </summary>
-        private Regex(string pattern, bool addToCache) =>
-            Init(pattern, RegexOptions.None, s_defaultMatchTimeout, addToCache);
+        public Regex(string pattern, RegexOptions options) :
+            this(pattern, options, s_defaultMatchTimeout, culture: null)
+        {
+        }
 
-        /// <summary>
-        /// Creates and compiles a regular expression object for the
-        /// specified regular expression with options that modify the pattern.
-        /// </summary>
-        public Regex(string pattern, RegexOptions options) =>
-            InitWithPossibleCompilation(pattern, options, s_defaultMatchTimeout, addToCache: false);
+        public Regex(string pattern, RegexOptions options, TimeSpan matchTimeout) :
+            this(pattern, options, matchTimeout, culture: null)
+        {
+        }
+
+        internal Regex(string pattern, CultureInfo? culture)
+        {
+            // Call Init directly rather than delegating to a Regex ctor that takes
+            // options to avoid rooting the Regex compiler unless necessary.
+            Init(pattern, RegexOptions.None, s_defaultMatchTimeout, culture);
+        }
 
-        public Regex(string pattern, RegexOptions options, TimeSpan matchTimeout) =>
-            InitWithPossibleCompilation(pattern, options, matchTimeout, addToCache: false);
+        internal Regex(string pattern, RegexOptions options, TimeSpan matchTimeout, CultureInfo? culture)
+        {
+            Init(pattern, options, matchTimeout, culture);
 
-        private Regex(string pattern, RegexOptions options, TimeSpan matchTimeout, bool addToCache) =>
-            InitWithPossibleCompilation(pattern, options, matchTimeout, addToCache);
+#if FEATURE_COMPILED
+            // if the compile option is set, then compile the code
+            if (UseOptionC())
+            {
+                // Storing into this Regex's factory will also implicitly update the cache
+                factory = Compile(_code!, options, matchTimeout != InfiniteMatchTimeout);
+                _code = null;
+            }
+#endif
+        }
 
         /// <summary>Initializes the instance.</summary>
         /// <remarks>
         /// This is separated out of the constructor to allow the Regex ctor that doesn't
         /// take a RegexOptions to avoid rooting the regex compiler, such that it can be trimmed away.
-        /// If <paramref name="options"/> may possibly include RegexOptions.Compiled, InitWithPossibleCompilation
-        /// must be invoked instead.
         /// </remarks>
-        private CachedCodeEntry? Init(string pattern, RegexOptions options, TimeSpan matchTimeout, bool addToCache)
+        private void Init(string pattern, RegexOptions options, TimeSpan matchTimeout, CultureInfo? culture)
+        {
+            ValidatePattern(pattern);
+            ValidateOptions(options);
+            ValidateMatchTimeout(matchTimeout);
+
+            this.pattern = pattern;
+            roptions = options;
+            internalMatchTimeout = matchTimeout;
+
+            // Parse the input
+            RegexTree tree = RegexParser.Parse(pattern, roptions, culture ?? ((options & RegexOptions.CultureInvariant) != 0 ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture));
+
+            // Extract the relevant information
+            capnames = tree.CapNames;
+            capslist = tree.CapsList;
+            _code = RegexWriter.Write(tree);
+            caps = _code.Caps;
+            capsize = _code.CapSize;
+
+            InitializeReferences();
+        }
+
+        internal static void ValidatePattern(string pattern)
         {
-            if (pattern == null)
+            if (pattern is null)
             {
                 throw new ArgumentNullException(nameof(pattern));
             }
+        }
 
+        internal static void ValidateOptions(RegexOptions options)
+        {
             if (options < RegexOptions.None || (((int)options) >> MaxOptionShift) != 0)
             {
                 throw new ArgumentOutOfRangeException(nameof(options));
             }
 
-            if ((options & RegexOptions.ECMAScript) != 0
-             && (options & ~(RegexOptions.ECMAScript |
+            if ((options & RegexOptions.ECMAScript) != 0 &&
+                (options & ~(RegexOptions.ECMAScript |
                              RegexOptions.IgnoreCase |
                              RegexOptions.Multiline |
                              RegexOptions.Compiled |
-                             RegexOptions.CultureInvariant
 #if DEBUG
-                           | RegexOptions.Debug
+                             RegexOptions.Debug |
 #endif
-                                               )) != 0)
+                             RegexOptions.CultureInvariant)) != 0)
             {
                 throw new ArgumentOutOfRangeException(nameof(options));
             }
-
-            ValidateMatchTimeout(matchTimeout);
-
-            // After parameter validation assign
-            this.pattern = pattern;
-            roptions = options;
-            internalMatchTimeout = matchTimeout;
-
-            // Cache handling. Try to look up this regex in the cache.
-            CultureInfo culture = (options & RegexOptions.CultureInvariant) != 0 ?
-                CultureInfo.InvariantCulture :
-                CultureInfo.CurrentCulture;
-            var key = new CachedCodeEntryKey(pattern, culture.ToString(), options, matchTimeout != InfiniteMatchTimeout);
-            CachedCodeEntry? cached = GetCachedCode(key, isToAdd: false);
-
-            if (cached == null)
-            {
-                // Parse the input
-                RegexTree? tree = RegexParser.Parse(pattern, roptions, culture);
-
-                // Extract the relevant information
-                capnames = tree.CapNames;
-                capslist = tree.CapsList;
-                _code = RegexWriter.Write(tree);
-                caps = _code.Caps;
-                capsize = _code.CapSize;
-
-                InitializeReferences();
-
-                tree = null;
-                if (addToCache)
-                    cached = GetCachedCode(key, true);
-            }
-            else
-            {
-                caps = cached.Caps;
-                capnames = cached.Capnames;
-                capslist = cached.Capslist;
-                capsize = cached.Capsize;
-                _code = cached.Code;
-#if FEATURE_COMPILED
-                factory = cached.Factory;
-#endif
-
-                // Cache runner and replacement
-                _runnerref = cached.Runnerref;
-                _replref = cached.ReplRef;
-                _refsInitialized = true;
-            }
-
-            return cached;
         }
 
-        /// <summary>Initializes the instance.</summary>
-        private void InitWithPossibleCompilation(string pattern, RegexOptions options, TimeSpan matchTimeout, bool addToCache)
+        /// <summary>
+        /// Validates that the specified match timeout value is valid.
+        /// The valid range is <code>TimeSpan.Zero &lt; matchTimeout &lt;= Regex.MaximumMatchTimeout</code>.
+        /// </summary>
+        /// <param name="matchTimeout">The timeout value to validate.</param>
+        /// <exception cref="ArgumentOutOfRangeException">If the specified timeout is not within a valid range.
+        /// </exception>
+        protected internal static void ValidateMatchTimeout(TimeSpan matchTimeout)
         {
-            CachedCodeEntry? cached = Init(pattern, options, matchTimeout, addToCache);
+            if (InfiniteMatchTimeout == matchTimeout)
+                return;
 
-#if FEATURE_COMPILED
-            // if the compile option is set, then compile the code if it's not already
-            if (UseOptionC() && factory == null)
-            {
-                factory = Compile(_code!, roptions, matchTimeout != InfiniteMatchTimeout);
-
-                if (addToCache && cached != null)
-                {
-                    cached.AddCompiled(factory);
-                }
+            // make sure timeout is not longer then Environment.Ticks cycle length:
+            if (TimeSpan.Zero < matchTimeout && matchTimeout <= s_maximumMatchTimeout)
+                return;
 
-                _code = null;
-            }
-#endif
+            throw new ArgumentOutOfRangeException(nameof(matchTimeout));
         }
 
         protected Regex(SerializationInfo info, StreamingContext context) =>
@@ -217,9 +198,9 @@ namespace System.Text.RegularExpressions
         /// instantiating a non-compiled regex.
         /// </summary>
         [MethodImpl(MethodImplOptions.NoInlining)]
-        private RegexRunnerFactory Compile(RegexCode code, RegexOptions roptions, bool hasTimeout)
+        private static RegexRunnerFactory Compile(RegexCode code, RegexOptions options, bool hasTimeout)
         {
-            return RegexCompiler.Compile(code!, roptions, hasTimeout);
+            return RegexCompiler.Compile(codeoptions, hasTimeout);
         }
 #endif
 
index 169e548..03de6ce 100644 (file)
@@ -137,7 +137,8 @@ namespace System.Text.RegularExpressions.Tests
 
         private int GetCachedItemsNum()
         {
-            return (int)typeof(Regex)
+            return (int)typeof(Regex).Assembly
+                .GetType("System.Text.RegularExpressions.RegexCache")
                 .GetField("s_cacheCount", BindingFlags.NonPublic | BindingFlags.Static)
                 .GetValue(null);
         }