summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDinnerbone <dinnerbone@dinnerbone.com>2011-07-15 18:28:09 +0100
committerDinnerbone <dinnerbone@dinnerbone.com>2011-07-15 20:52:54 +0100
commita507add4ea94d2000f67285029eb8c3f9298215c (patch)
treef9b6bf58bc7e158af23304976fcebd9d7b310563
parent0bd535e054b50ede75da54e55f1c1c5487604a4f (diff)
downloadcraftbukkit-a507add4ea94d2000f67285029eb8c3f9298215c.tar
craftbukkit-a507add4ea94d2000f67285029eb8c3f9298215c.tar.gz
craftbukkit-a507add4ea94d2000f67285029eb8c3f9298215c.tar.lz
craftbukkit-a507add4ea94d2000f67285029eb8c3f9298215c.tar.xz
craftbukkit-a507add4ea94d2000f67285029eb8c3f9298215c.zip
Fixed huge memory leak (gigabytes/hour!) by placing a bukkit under the ceiling.
-rw-r--r--pom.xml5
-rw-r--r--src/main/java/org/bukkit/craftbukkit/CraftChunk.java5
-rw-r--r--src/main/java/org/bukkit/craftbukkit/CraftWorld.java5
-rw-r--r--src/main/java/org/bukkit/craftbukkit/util/ConcurrentSoftMap.java268
4 files changed, 11 insertions, 272 deletions
diff --git a/pom.xml b/pom.xml
index 7917f3db..782053d1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -79,6 +79,11 @@
<type>jar</type>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava-collections</artifactId>
+ <version>r03</version>
+ </dependency>
</dependencies>
<!-- This builds a completely 'ready to start' jar with all dependencies inside -->
<build>
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftChunk.java b/src/main/java/org/bukkit/craftbukkit/CraftChunk.java
index 39512e95..2102250d 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftChunk.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftChunk.java
@@ -1,6 +1,8 @@
package org.bukkit.craftbukkit;
+import com.google.common.collect.MapMaker;
import java.lang.ref.WeakReference;
+import java.util.concurrent.ConcurrentMap;
import net.minecraft.server.ChunkPosition;
import net.minecraft.server.WorldServer;
@@ -11,14 +13,13 @@ import org.bukkit.block.Block;
import org.bukkit.block.BlockState;
import org.bukkit.craftbukkit.block.CraftBlock;
import org.bukkit.entity.Entity;
-import org.bukkit.craftbukkit.util.ConcurrentSoftMap;
import org.bukkit.ChunkSnapshot;
import net.minecraft.server.BiomeBase;
import net.minecraft.server.WorldChunkManager;
public class CraftChunk implements Chunk {
private WeakReference<net.minecraft.server.Chunk> weakChunk;
- private final ConcurrentSoftMap<Integer, Block> cache = new ConcurrentSoftMap<Integer, Block>();
+ private final ConcurrentMap<Integer, Block> cache = new MapMaker().softKeys().softValues().makeMap();
private WorldServer worldServer;
private int x;
private int z;
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
index 8c47fa75..bbda48d7 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -1,11 +1,12 @@
package org.bukkit.craftbukkit;
+import com.google.common.collect.MapMaker;
import org.bukkit.craftbukkit.entity.*;
import org.bukkit.entity.*;
import org.bukkit.entity.Entity;
import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentMap;
import java.util.List;
import java.util.Random;
import java.util.UUID;
@@ -37,7 +38,7 @@ public class CraftWorld implements World {
private final WorldServer world;
private Environment environment;
private final CraftServer server = (CraftServer)Bukkit.getServer();
- private HashMap<Integer, CraftChunk> unloadedChunks = new HashMap<Integer, CraftChunk>();
+ private ConcurrentMap<Integer, CraftChunk> unloadedChunks = new MapMaker().weakKeys().weakValues().makeMap();
private final ChunkGenerator generator;
private final List<BlockPopulator> populators = new ArrayList<BlockPopulator>();
diff --git a/src/main/java/org/bukkit/craftbukkit/util/ConcurrentSoftMap.java b/src/main/java/org/bukkit/craftbukkit/util/ConcurrentSoftMap.java
deleted file mode 100644
index 674f9bd1..00000000
--- a/src/main/java/org/bukkit/craftbukkit/util/ConcurrentSoftMap.java
+++ /dev/null
@@ -1,268 +0,0 @@
-package org.bukkit.craftbukkit.util;
-
-import java.util.Map;
-import java.util.LinkedList;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.Set;
-import java.util.Collection;
-import java.util.Iterator;
-
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.SoftReference;
-
-/**
- * Creates a map that uses soft reference. This indicates to the garbage collector
- * that they can be removed if necessary
- *
- * A minimum number of strong references can be set. These most recent N objects added
- * to the map will not be removed by the garbage collector.
- *
- * Objects will never be removed if they are referenced strongly from somewhere else
-
- * Note: While data corruption won't happen, the garbage collector is potentially async
- * This could lead to the return values from containsKey() and similar methods being
- * out of date by the time they are used. The class could return null when the object
- * is retrieved by a .get() call directly after a .containsKey() call returned true
- *
- * @author raphfrk
- */
-
-public class ConcurrentSoftMap<K, V> {
-
- private final ConcurrentHashMap<K, SoftMapReference<K, V>> map = new ConcurrentHashMap<K, SoftMapReference<K, V>>();
- private final ReferenceQueue<SoftMapReference> queue = new ReferenceQueue<SoftMapReference>();
- private final LinkedList<V> strongReferenceQueue = new LinkedList<V>();
- private final int strongReferenceSize;
-
- public ConcurrentSoftMap() {
- this(20);
- }
-
- public ConcurrentSoftMap(int size) {
- strongReferenceSize = size;
- }
-
- // When a soft reference is deleted by the garbage collector, it is set to reference null
- // and added to the queue
- //
- // However, these null references still exist in the ConcurrentHashMap as keys. This method removes these keys.
- //
- // It is called whenever there is a method call of the map.
-
- private void emptyQueue() {
- SoftMapReference ref;
-
- while ((ref = (SoftMapReference) queue.poll()) != null) {
- map.remove(ref.key);
- }
- }
-
- public void clear() {
- synchronized (strongReferenceQueue) {
- strongReferenceQueue.clear();
- }
- map.clear();
- emptyQueue();
- }
-
- // Shouldn't support this, since the garbage collection is async
-
- public boolean containsKey(K key) {
- emptyQueue();
- return map.containsKey(key);
- }
-
- // Shouldn't support this, since the garbage collection is async
-
- public boolean containsValue(V value) {
- emptyQueue();
- return map.containsValue(value);
- }
-
- // Shouldn't support this since it would create strong references to all the entries
-
- public Set entrySet() {
- emptyQueue();
- throw new UnsupportedOperationException("SoftMap does not support this operation, since it creates potentially stong references");
- }
-
- // Doesn't support these either
-
- public boolean equals(Object o) {
- emptyQueue();
- throw new UnsupportedOperationException("SoftMap doesn't support equals checks");
- }
-
- // This operation returns null if the entry is not in the map
-
- public V get(K key) {
- emptyQueue();
- return fastGet(key);
- }
-
- private V fastGet(K key) {
- SoftMapReference<K, V> ref = map.get(key);
-
- if (ref == null) {
- return null;
- }
- V value = ref.get();
-
- if (value != null) {
- synchronized (strongReferenceQueue) {
- strongReferenceQueue.addFirst(value);
- if (strongReferenceQueue.size() > strongReferenceSize) {
- strongReferenceQueue.removeLast();
- }
- }
- }
- return value;
- }
-
- // Doesn't support this either
-
- public int hashCode() {
- emptyQueue();
- throw new UnsupportedOperationException("SoftMap doesn't support hashCode");
- }
-
- // This is another risky method, since again, garbage collection is async
-
- public boolean isEmpty() {
- emptyQueue();
- return map.isEmpty();
- }
-
- // Return all the keys, again could go out of date
-
- public Set keySet() {
- emptyQueue();
- return map.keySet();
- }
-
- // Adds the mapping to the map
-
- public V put(K key, V value) {
- emptyQueue();
- V old = fastGet(key);
- fastPut(key, value);
- return old;
- }
-
- private void fastPut(K key, V value) {
- map.put(key, new SoftMapReference<K, V>(key, value, queue));
- synchronized (strongReferenceQueue) {
- strongReferenceQueue.addFirst(value);
- if (strongReferenceQueue.size() > strongReferenceSize) {
- strongReferenceQueue.removeLast();
- }
- }
- }
-
- public V putIfAbsent(K key, V value) {
- emptyQueue();
- return fastPutIfAbsent(key, value);
- }
-
- private V fastPutIfAbsent(K key, V value) {
- V ret = null;
-
- if (map.containsKey(key)) {
- SoftMapReference<K, V> current = map.get(key);
-
- if (current != null) {
- ret = current.get();
- }
- }
-
- if (ret == null) {
- SoftMapReference<K, V> newValue = new SoftMapReference<K, V>(key, value, queue);
- boolean success = false;
-
- while (!success) {
- SoftMapReference<K, V> oldValue = map.putIfAbsent(key, newValue);
-
- if (oldValue == null) { // put was successful (key didn't exist)
- ret = null;
- success = true;
- } else {
- ret = oldValue.get();
- if (ret == null) { // key existed, but referenced null
- success = map.replace(key, oldValue, newValue); // try to swap old for new
- } else { // key existed, and referenced a valid object
- success = true;
- }
- }
- }
- }
-
- if (ret == null) {
- synchronized (strongReferenceQueue) {
- strongReferenceQueue.addFirst(value);
- if (strongReferenceQueue.size() > strongReferenceSize) {
- strongReferenceQueue.removeLast();
- }
- }
- }
-
- return ret;
- }
-
- // Adds the mappings to the map
-
- public void putAll(Map other) {
- emptyQueue();
- Iterator<K> itr = other.keySet().iterator();
- while (itr.hasNext()) {
- K key = itr.next();
- fastPut(key, (V) other.get(key));
- }
- }
-
- // Remove object
-
- public V remove(K key) {
- emptyQueue();
- SoftMapReference<K, V> ref = map.remove(key);
-
- if (ref != null) {
- return ref.get();
- }
- return null;
- }
-
- // Returns size, could go out of date
-
- public int size() {
- emptyQueue();
- return map.size();
- }
-
- // Shouldn't support this since it would create strong references to all the entries
-
- public Collection values() {
- emptyQueue();
- throw new UnsupportedOperationException("SoftMap does not support this operation, since it creates potentially stong references");
- }
-
- private static class SoftMapReference<K, V> extends SoftReference<V> {
- K key;
-
- SoftMapReference(K key, V value, ReferenceQueue queue) {
- super(value, queue);
- this.key = key;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o == null) {
- return false;
- }
- if (!(o instanceof SoftMapReference)) {
- return false;
- }
- SoftMapReference other = (SoftMapReference) o;
- return other.get() == get();
- }
- }
-}