Remove custom regex cache and just use a weak-valued map. #2078.

Author: headius

core/src/main/java/org/jruby/RubyRegexp.java

@@ -38,11 +38,6 @@
 import static org.jruby.anno.FrameField.BACKREF;
 import static org.jruby.anno.FrameField.LASTLINE;
 
-import java.lang.ref.SoftReference;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-
 import org.jcodings.Encoding;
 import org.jcodings.specific.ASCIIEncoding;
 import org.jcodings.specific.USASCIIEncoding;
@@ -78,6 +73,9 @@
 import org.jruby.util.Sprintf;
 import org.jruby.util.StringSupport;
 import org.jruby.util.TypeConverter;
+import org.jruby.util.collections.WeakValuedMap;
+
+import java.util.Iterator;
 
 @JRubyClass(name="Regexp")
 public class RubyRegexp extends RubyObject implements ReOptions, EncodingCapable, MarshalEncoding {
@@ -140,22 +138,10 @@ public boolean shouldMarshalEncoding() {
     public Encoding getMarshalEncoding() {
         return getEncoding();
     }
-
-    private static final class RegexpCache {
-        private volatile SoftReference<Map<ByteList, Regex>> cache = new SoftReference<Map<ByteList, Regex>>(null);
-        private Map<ByteList, Regex> get() {
-            Map<ByteList, Regex> patternCache = cache.get();
-            if (patternCache == null) {
-                patternCache = new ConcurrentHashMap<ByteList, Regex>(5);
-                cache = new SoftReference<Map<ByteList, Regex>>(patternCache);
-            }
-            return patternCache;
-        }
-    }
-
-    private static final RegexpCache patternCache = new RegexpCache();
-    private static final RegexpCache quotedPatternCache = new RegexpCache();
-    private static final RegexpCache preprocessedPatternCache = new RegexpCache();
+    // FIXME: Maybe these should not be static?
+    private static final WeakValuedMap<ByteList, Regex> patternCache = new WeakValuedMap();
+    private static final WeakValuedMap<ByteList, Regex> quotedPatternCache = new WeakValuedMap();
+    private static final WeakValuedMap<ByteList, Regex> preprocessedPatternCache = new WeakValuedMap();
 
     private static Regex makeRegexp(Ruby runtime, ByteList bytes, RegexpOptions options, Encoding enc) {
         try {
@@ -172,46 +158,42 @@ private static Regex makeRegexp(Ruby runtime, ByteList bytes, RegexpOptions opti
     }
 
     static Regex getRegexpFromCache(Ruby runtime, ByteList bytes, Encoding enc, RegexpOptions options) {
-        Map<ByteList, Regex> cache = patternCache.get();
-        Regex regex = cache.get(bytes);
+        Regex regex = patternCache.get(bytes);
         if (regex != null && regex.getEncoding() == enc && regex.getOptions() == options.toJoniOptions()) return regex;
         regex = makeRegexp(runtime, bytes, options, enc);
         regex.setUserObject(bytes);
-        cache.put(bytes, regex);
+        patternCache.put(bytes, regex);
         return regex;
     }
 
     static Regex getQuotedRegexpFromCache(Ruby runtime, ByteList bytes, Encoding enc, RegexpOptions options) {
-        Map<ByteList, Regex> cache = quotedPatternCache.get();
-        Regex regex = cache.get(bytes);
+        Regex regex = quotedPatternCache.get(bytes);
         if (regex != null && regex.getEncoding() == enc && regex.getOptions() == options.toJoniOptions()) return regex;
         ByteList quoted = quote(bytes, enc);
         regex = makeRegexp(runtime, quoted, options, enc);
         regex.setUserObject(quoted);
-        cache.put(bytes, regex);
+        quotedPatternCache.put(bytes, regex);
         return regex;
     }
 
     static Regex getQuotedRegexpFromCache19(Ruby runtime, ByteList bytes, RegexpOptions options, boolean asciiOnly) {
-        Map<ByteList, Regex> cache = quotedPatternCache.get();
-        Regex regex = cache.get(bytes);
+        Regex regex = quotedPatternCache.get(bytes);
         Encoding enc = asciiOnly ? USASCIIEncoding.INSTANCE : bytes.getEncoding();
         if (regex != null && regex.getEncoding() == enc && regex.getOptions() == options.toJoniOptions()) return regex;
         ByteList quoted = quote19(bytes, asciiOnly);
         regex = makeRegexp(runtime, quoted, options, quoted.getEncoding());
         regex.setUserObject(quoted);
-        cache.put(bytes, regex);
+        quotedPatternCache.put(bytes, regex);
         return regex;
     }
 
     private static Regex getPreprocessedRegexpFromCache(Ruby runtime, ByteList bytes, Encoding enc, RegexpOptions options, ErrorMode mode) {
-        Map<ByteList, Regex> cache = preprocessedPatternCache.get();
-        Regex regex = cache.get(bytes);
+        Regex regex = preprocessedPatternCache.get(bytes);
         if (regex != null && regex.getEncoding() == enc && regex.getOptions() == options.toJoniOptions()) return regex;
         ByteList preprocessed = preprocess(runtime, bytes, enc, new Encoding[]{null}, ErrorMode.RAISE);
         regex = makeRegexp(runtime, preprocessed, options, enc);
         regex.setUserObject(preprocessed);
-        cache.put(bytes, regex);
+        preprocessedPatternCache.put(bytes, regex);
         return regex;
     }
 

core/src/main/java/org/jruby/util/collections/WeakValuedIdentityMap.java

@@ -32,47 +32,15 @@
 
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
+import java.util.Collections;
 import java.util.IdentityHashMap;
 import java.util.Map;
 
 /**
  * A Map that holds its values weakly and uses object identity for keys.
  */
-public class WeakValuedIdentityMap<Key, Value> {
-    private final ReferenceQueue deadReferences = new ReferenceQueue();
-    private final Map<Key, KeyedReference<Key, Value>> references = new IdentityHashMap<Key, KeyedReference<Key, Value>>();
-
-    public synchronized void put(Key key, Value value) {
-        cleanReferences();
-        references.put(key, new KeyedReference(value, key, deadReferences));
-    }
-
-    public synchronized Value get(Key key) {
-        cleanReferences();
-        KeyedReference<Key, Value> reference = references.get(key);
-        if (reference == null) {
-            return null;
-        }
-        return reference.get();
-    }
-
-    private void cleanReferences() {
-        KeyedReference ref;
-        while ((ref = (KeyedReference) deadReferences.poll()) != null) {
-            references.remove((ref.key()));
-        }
-    }
-
-    private static class KeyedReference<Key, Value> extends WeakReference<Value> {
-        private final Key key;
-
-        public KeyedReference(Value object, Key key, ReferenceQueue queue) {
-            super(object, queue);
-            this.key = key;
-        }
-
-        public Key key() {
-            return key;
-        }
+public class WeakValuedIdentityMap<Key, Value> extends WeakValuedMap<Key, Value> {
+    protected Map<Key, KeyedReference<Key, Value>> newMap() {
+        return Collections.synchronizedMap(new IdentityHashMap());
     }
 }

core/src/main/java/org/jruby/util/collections/WeakValuedMap.java

@@ -0,0 +1,83 @@
+/***** BEGIN LICENSE BLOCK *****
+ * Version: EPL 1.0/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Eclipse Public
+ * License Version 1.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ *
+ * Copyright (C) 2002-2004 Jan Arne Petersen <jpetersen@uni-bonn.de>
+ * Copyright (C) 2002-2004 Anders Bengtsson <ndrsbngtssn@yahoo.se>
+ * Copyright (C) 2004-2006 Charles O Nutter <headius@headius.com>
+ * Copyright (C) 2004 Stefan Matthias Aust <sma@3plus4.de>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the EPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the EPL, the GPL or the LGPL.
+ ***** END LICENSE BLOCK *****/
+package org.jruby.util.collections;
+
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * A Map that holds its values weakly and uses object identity for keys.
+ */
+public class WeakValuedMap<Key, Value> {
+    private final ReferenceQueue deadReferences = new ReferenceQueue();
+    private final Map<Key, KeyedReference<Key, Value>> references = newMap();
+
+    public void put(Key key, Value value) {
+        cleanReferences();
+        references.put(key, new KeyedReference(value, key, deadReferences));
+    }
+
+    public Value get(Key key) {
+        cleanReferences();
+        KeyedReference<Key, Value> reference = references.get(key);
+        if (reference == null) {
+            return null;
+        }
+        return reference.get();
+    }
+
+    protected Map<Key, KeyedReference<Key, Value>> newMap() {
+        return new ConcurrentHashMap();
+    }
+
+    protected static class KeyedReference<Key, Value> extends WeakReference<Value> {
+        private final Key key;
+
+        public KeyedReference(Value object, Key key, ReferenceQueue queue) {
+            super(object, queue);
+            this.key = key;
+        }
+
+        public Key key() {
+            return key;
+        }
+    }
+
+    private void cleanReferences() {
+        KeyedReference ref;
+        while ((ref = (KeyedReference) deadReferences.poll()) != null) {
+            references.remove((ref.key()));
+        }
+    }
+}