Improve speed and memory use of FixedMetadataValue. Fixes BUKKIT-1460

FixedMetadataValue currently just extends LazyMetadataValue with a value
that never changes. While this works it is a lot of unneeded overhead
that causes FixedMetadataValue to be a lot slower and use a lot more
memory than one would expect. To correct this we store the value directly
in FixedMetadataValue and override the the appropriate methods to use it.

Ideally we would modify FixedMetadataValue to no longer extend
LazyMetadataValue as this would give a very large memory savings. However,
this is not currently done for backwards compatibility reasons.

3 files changed, 51 insertions, 21 deletions

diff --git a/src/main/java/org/bukkit/metadata/FixedMetadataValue.java b/src/main/java/org/bukkit/metadata/FixedMetadataValue.java
index 41f89a0e..be1c5aa0 100644
--- a/src/main/java/org/bukkit/metadata/FixedMetadataValue.java
+++ b/src/main/java/org/bukkit/metadata/FixedMetadataValue.java
@@ -6,9 +6,15 @@ import java.util.concurrent.Callable;
 
 /**
  * A FixedMetadataValue is a special case metadata item that contains the same value forever after initialization.
- * Invalidating a FixedMetadataValue has no affect.
+ * Invalidating a FixedMetadataValue has no effect.
+ *
+ * This class extends LazyMetadataValue for historical reasons, even though it overrides all the implementation
+ * methods. it is possible that in the future that the inheritance hierarchy may change.
  */
 public class FixedMetadataValue extends LazyMetadataValue {
+    /** Store the internal value that is represented by this fixed value. */
+    private final Object internalValue;
+
     /**
      * Initializes a FixedMetadataValue with an Object
      *
@@ -16,10 +22,17 @@ public class FixedMetadataValue extends LazyMetadataValue {
      * @param value the value assigned to this metadata value.
      */
     public FixedMetadataValue(Plugin owningPlugin, final Object value) {
-        super(owningPlugin, CacheStrategy.CACHE_ETERNALLY, new Callable<Object>() {
-            public Object call() throws Exception {
-                return value;
-            }
-        });
+        super(owningPlugin);
+        this.internalValue = value;
+    }
+
+    @Override
+    public void invalidate() {
+
+    }
+
+    @Override
+    public Object value() {
+        return internalValue;
     }
 }
diff --git a/src/main/java/org/bukkit/metadata/LazyMetadataValue.java b/src/main/java/org/bukkit/metadata/LazyMetadataValue.java
index 57fdc502..d67e633c 100644
--- a/src/main/java/org/bukkit/metadata/LazyMetadataValue.java
+++ b/src/main/java/org/bukkit/metadata/LazyMetadataValue.java
@@ -16,7 +16,7 @@ import org.bukkit.plugin.Plugin;
 public class LazyMetadataValue extends MetadataValueAdapter implements MetadataValue {
     private Callable<Object> lazyValue;
     private CacheStrategy cacheStrategy;
-    private SoftReference<Object> internalValue = new SoftReference<Object>(null);
+    private SoftReference<Object> internalValue;
     private static final Object ACTUALLY_NULL = new Object();
 
     /**
@@ -40,11 +40,16 @@ public class LazyMetadataValue extends MetadataValueAdapter implements MetadataV
         super(owningPlugin);
         Validate.notNull(cacheStrategy, "cacheStrategy cannot be null");
         Validate.notNull(lazyValue, "lazyValue cannot be null");
-
+        this.internalValue = new SoftReference<Object>(null);
         this.lazyValue = lazyValue;
         this.cacheStrategy = cacheStrategy;
     }
 
+    /** Protected special constructor used by FixedMetadataValue to bypass standard setup. */
+    protected LazyMetadataValue(Plugin owningPlugin) {
+        super(owningPlugin);
+    }
+
     public Object value() {
         eval();
         Object value = internalValue.get();
diff --git a/src/test/java/org/bukkit/metadata/FixedMetadataValueTest.java b/src/test/java/org/bukkit/metadata/FixedMetadataValueTest.java
index 405169bc..5583b274 100644
--- a/src/test/java/org/bukkit/metadata/FixedMetadataValueTest.java
+++ b/src/test/java/org/bukkit/metadata/FixedMetadataValueTest.java
@@ -1,6 +1,7 @@
 package org.bukkit.metadata;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
 
 import org.bukkit.plugin.Plugin;
 import org.bukkit.plugin.TestPlugin;
@@ -10,21 +11,32 @@ public class FixedMetadataValueTest {
     private Plugin plugin = new TestPlugin("X");
     private FixedMetadataValue subject;
 
-    private void valueEquals(Object value) {
-        subject = new FixedMetadataValue(plugin, value);
-        assertEquals(value, subject.value());
+    @Test
+    public void testBasic() {
+        subject = new FixedMetadataValue(plugin, new Integer(50));
+        assertSame(plugin, subject.getOwningPlugin());
+        assertEquals(new Integer(50), subject.value());
+    }
+
+    @Test
+    public void testNumberTypes() {
+        subject = new FixedMetadataValue(plugin, new Integer(5));
+        assertEquals(new Integer(5), subject.value());
+        assertEquals(5, subject.asInt());
+        assertEquals(true, subject.asBoolean());
+        assertEquals(5, subject.asByte());
+        assertEquals(5.0, subject.asFloat(), 0.1e-8);
+        assertEquals(5.0D, subject.asDouble(), 0.1e-8D);
+        assertEquals(5L, subject.asLong());
+        assertEquals(5, subject.asShort());
+        assertEquals("5", subject.asString());
     }
 
     @Test
-    public void testTypes() {
-        valueEquals(10);
-        valueEquals(0.1);
-        valueEquals("TEN");
-        valueEquals(true);
-        valueEquals(null);
-        valueEquals((float) 10.5);
-        valueEquals((long) 10);
-        valueEquals((short) 10);
-        valueEquals((byte) 10);
+    public void testInvalidateDoesNothing() {
+        Object o = new Object();
+        subject = new FixedMetadataValue(plugin, o);
+        subject.invalidate();
+        assertSame(o, subject.value());
     }
 }