Don't cache metadata store disambiguations. Fixes BUKKIT-3841

The metadata system generates unique keys for metadata entries based on
the subject metadata is being applied to and the name of the metadata
being applied. It was assumed this would be an expensive operation so a
cache was put in place to ensure this was done as little as possible.

In reality this cache only has a benefit when you have a hit rate above
~90% and is otherwise much slower. As the implementation of the cache is
a hashmap of hashmaps it also uses a significant amount of memory which
is not worth it even for the performance increase with a high hit rate.

This commit simply removes the cache which results in speedups for most
cases and large memory savings.

1 files changed, 4 insertions, 28 deletions

diff --git a/src/main/java/org/bukkit/metadata/MetadataStoreBase.java b/src/main/java/org/bukkit/metadata/MetadataStoreBase.java
index 7febbd43..d6546a92 100644
--- a/src/main/java/org/bukkit/metadata/MetadataStoreBase.java
+++ b/src/main/java/org/bukkit/metadata/MetadataStoreBase.java
@@ -7,7 +7,6 @@ import java.util.*;
 
 public abstract class MetadataStoreBase<T> {
     private Map<String, List<MetadataValue>> metadataMap = new HashMap<String, List<MetadataValue>>();
-    private WeakHashMap<T, Map<String, String>> disambiguationCache = new WeakHashMap<T, Map<String, String>>();
 
     /**
      * Adds a metadata value to an object. Each metadata value is owned by a specific{@link Plugin}.
@@ -28,7 +27,7 @@ public abstract class MetadataStoreBase<T> {
     public synchronized void setMetadata(T subject, String metadataKey, MetadataValue newMetadataValue) {
         Validate.notNull(newMetadataValue, "Value cannot be null");
         Validate.notNull(newMetadataValue.getOwningPlugin(), "Plugin cannot be null");
-        String key = cachedDisambiguate(subject, metadataKey);
+        String key = disambiguate(subject, metadataKey);
         if (!metadataMap.containsKey(key)) {
             metadataMap.put(key, new ArrayList<MetadataValue>());
         }
@@ -55,7 +54,7 @@ public abstract class MetadataStoreBase<T> {
      * @see MetadataStore#getMetadata(Object, String)
      */
     public synchronized List<MetadataValue> getMetadata(T subject, String metadataKey) {
-        String key = cachedDisambiguate(subject, metadataKey);
+        String key = disambiguate(subject, metadataKey);
         if (metadataMap.containsKey(key)) {
             return Collections.unmodifiableList(metadataMap.get(key));
         } else {
@@ -71,7 +70,7 @@ public abstract class MetadataStoreBase<T> {
      * @return the existence of the metadataKey within subject.
      */
     public synchronized boolean hasMetadata(T subject, String metadataKey) {
-        String key = cachedDisambiguate(subject, metadataKey);
+        String key = disambiguate(subject, metadataKey);
         return metadataMap.containsKey(key);
     }
 
@@ -86,7 +85,7 @@ public abstract class MetadataStoreBase<T> {
      */
     public synchronized void removeMetadata(T subject, String metadataKey, Plugin owningPlugin) {
         Validate.notNull(owningPlugin, "Plugin cannot be null");
-        String key = cachedDisambiguate(subject, metadataKey);
+        String key = disambiguate(subject, metadataKey);
         List<MetadataValue> metadataList = metadataMap.get(key);
         if (metadataList == null) return;
         for (int i = 0; i < metadataList.size(); i++) {
@@ -119,29 +118,6 @@ public abstract class MetadataStoreBase<T> {
     }
 
     /**
-     * Caches the results of calls to {@link MetadataStoreBase#disambiguate(Object, String)} in a {@link WeakHashMap}. Doing so maintains a
-     * <a href="http://www.codeinstructions.com/2008/09/weakhashmap-is-not-cache-understanding.html">canonical list</a>
-     * of disambiguation strings for objects in memory. When those objects are garbage collected, the disambiguation string
-     * in the list is aggressively garbage collected as well.
-     *
-     * @param subject     The object for which this key is being generated.
-     * @param metadataKey The name identifying the metadata value.
-     * @return a unique metadata key for the given subject.
-     */
-    private String cachedDisambiguate(T subject, String metadataKey) {
-        if (disambiguationCache.containsKey(subject) && disambiguationCache.get(subject).containsKey(metadataKey)) {
-            return disambiguationCache.get(subject).get(metadataKey);
-        } else {
-            if (!disambiguationCache.containsKey(subject)) {
-                disambiguationCache.put(subject, new HashMap<String, String>());
-            }
-            String disambiguation = disambiguate(subject, metadataKey);
-            disambiguationCache.get(subject).put(metadataKey, disambiguation);
-            return disambiguation;
-        }
-    }
-
-    /**
      * Creates a unique name for the object receiving metadata by combining unique data from the subject with a metadataKey.
      * The name created must be globally unique for the given object and any two equivalent objects must generate the
      * same unique name. For example, two Player objects must generate the same string if they represent the same player,