Revert "Rework locking to try to reduce lag. Related to #1824"

This reverts commit 6fbeb49. This caused issues for some users, so it is being reverted in the interest of functionality.
PEXPlugins · Jun 21, 2015 · 2fe5a11 · 2fe5a11
1 parent 17405af
commit 2fe5a11
Show file tree

Hide file tree

Showing 6 changed files with 135 additions and 193 deletions.
diff --git a/src/main/java/ru/tehkode/permissions/backends/caching/CachingData.java b/src/main/java/ru/tehkode/permissions/backends/caching/CachingData.java
@@ -9,111 +9,73 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Executor;
-import java.util.concurrent.locks.ReadWriteLock;
 
 /**
  * Data backend implementing a simple cache
  */
 public abstract class CachingData implements PermissionsData {
 	private final Executor executor;
-	protected final ReadWriteLock lock;
+	protected final Object lock;
 	private Map<String, List<String>> permissions;
 	private Map<String, Map<String, String>> options;
 	private Map<String, List<String>> parents;
 	private volatile Set<String> worlds;
 
-	public CachingData(Executor executor, ReadWriteLock lock) {
+	public CachingData(Executor executor, Object lock) {
 		this.executor = executor;
 		this.lock = lock;
 	}
 
-	protected void executeWrite(final Runnable run) {
+	protected void execute(final Runnable run) {
 		executor.execute(new Runnable() {
 			@Override
 			public void run() {
-				lock.writeLock().lock();
-				try {
+				synchronized (lock) {
 					run.run();
-				} finally {
-					lock.readLock().lock();
-					try {
-						lock.writeLock().unlock();
-						getBackingData().save();
-					} finally {
-						lock.readLock().unlock();
-					}
 				}
 			}
 		});
 	}
-	protected void executeRead(final Runnable run) {
-		executor.execute(new Runnable() {
-			@Override
-			public void run() {
-				lock.readLock().lock();
-				try {
-					run.run();
-				} finally {
-					lock.readLock().unlock();
-				}
-			}
-		});
-		}
 
 	protected abstract PermissionsData getBackingData();
 
 	protected void loadPermissions() {
-		lock.readLock().lock();
-		try {
+		synchronized (lock) {
 			this.permissions = new HashMap<>(getBackingData().getPermissionsMap());
-		} finally {
-			lock.readLock().unlock();
 		}
 	}
 
 	protected void loadOptions() {
-		lock.readLock().lock();
-		try {
+		synchronized (lock) {
 			this.options = new HashMap<>();
 			for (Map.Entry<String, Map<String, String>> e : getBackingData().getOptionsMap().entrySet()) {
 				this.options.put(e.getKey(), new HashMap<>(e.getValue()));
 			}
-		} finally {
-			lock.readLock().unlock();
 		}
 	}
 
 	protected void loadInheritance() {
-		lock.readLock().lock();
-		try {
+		synchronized (lock) {
 			this.parents = new HashMap<>(getBackingData().getParentsMap());
-		} finally {
-			lock.readLock().unlock();
 		}
 	}
 
 	protected void clearCache() {
-		lock.writeLock().lock();
-		try {
+		synchronized (lock) {
 			permissions = null;
 			options = null;
 			parents = null;
 			clearWorldsCache();
-		} finally {
-			lock.writeLock().unlock();
 		}
 	}
 
 	@Override
 	public void load() {
-		lock.writeLock().lock();
-		try {
+		synchronized (lock) {
 			getBackingData().load();
 			loadInheritance();
 			loadOptions();
 			loadPermissions();
-		} finally {
-			lock.writeLock().unlock();
 		}
 	}
 
@@ -137,7 +99,7 @@ public void setPermissions(List<String> permissions, final String worldName) {
 			loadPermissions();
 		}
 		final List<String> safePermissions = new ArrayList<>(permissions);
-		executeWrite(new Runnable() {
+		execute(new Runnable() {
 			@Override
 			public void run() {
 				clearWorldsCache();
@@ -164,11 +126,8 @@ public Map<String, List<String>> getPermissionsMap() {
 	public Set<String> getWorlds() {
 		Set<String> worlds = this.worlds;
 		if (worlds == null) {
-			lock.readLock().lock();
-			try {
+			synchronized (lock) {
 				this.worlds = worlds = getBackingData().getWorlds();
-			} finally {
-				lock.readLock().unlock();
 			}
 		}
 		return worlds;
@@ -195,26 +154,26 @@ public void setOption(final String option, final String value, final String worl
 		if (options == null) {
 			loadOptions();
 		}
-		executeWrite(new Runnable() {
+		execute(new Runnable() {
 			@Override
 			public void run() {
-				if (options != null) {
-					Map<String, String> optionsMap = options.get(world);
-					if (optionsMap == null) {
-						// TODO Concurrentify
-						optionsMap = new HashMap<>();
-						options.put(world, optionsMap);
-						clearWorldsCache();
-					}
-					if (value == null) {
-						optionsMap.remove(option);
-					} else {
-						optionsMap.put(option, value);
-					}
-				}
 				getBackingData().setOption(option, value, world);
 			}
 		});
+		if (options != null) {
+			Map<String, String> optionsMap = options.get(world);
+			if (optionsMap == null) {
+				// TODO Concurrentify
+				optionsMap = new HashMap<>();
+				options.put(world, optionsMap);
+				clearWorldsCache();
+			}
+			if (value == null) {
+				optionsMap.remove(option);
+			} else {
+				optionsMap.put(option, value);
+			}
+		}
 	}
 
 	@Override
@@ -254,13 +213,13 @@ public void setParents(final List<String> rawParents, final String worldName) {
 			loadInheritance();
 		}
 		final List<String> safeParents = new ArrayList<>(rawParents);
-		executeWrite(new Runnable() {
+		execute(new Runnable() {
 			@Override
 			public void run() {
-				parents.put(worldName, Collections.unmodifiableList(safeParents));
 				getBackingData().setParents(safeParents, worldName);
 			}
 		});
+		this.parents.put(worldName, Collections.unmodifiableList(safeParents));
 	}
 
 	@Override
@@ -270,7 +229,7 @@ public boolean isVirtual() {
 
 	@Override
 	public void save() {
-		executeRead(new Runnable() {
+		execute(new Runnable() {
 			@Override
 			public void run() {
 				getBackingData().save();
@@ -280,12 +239,9 @@ public void run() {
 
 	@Override
 	public void remove() {
-		lock.writeLock().lock();
-		try {
+		synchronized (lock) {
 			getBackingData().remove();
 			clearCache();
-		} finally {
-			lock.writeLock().unlock();
 		}
 	}
 

diff --git a/src/main/java/ru/tehkode/permissions/backends/caching/CachingGroupData.java b/src/main/java/ru/tehkode/permissions/backends/caching/CachingGroupData.java
@@ -3,14 +3,13 @@
 import ru.tehkode.permissions.PermissionsGroupData;
 
 import java.util.concurrent.Executor;
-import java.util.concurrent.locks.ReadWriteLock;
 
 /**
  * Cached data for groups
  */
 public class CachingGroupData extends CachingData implements PermissionsGroupData {
 	private final PermissionsGroupData backingData;
-	public CachingGroupData(PermissionsGroupData backingData, Executor executor, ReadWriteLock lock) {
+	public CachingGroupData(PermissionsGroupData backingData, Executor executor, Object lock) {
 		super(executor, lock);
 		this.backingData = backingData;
 	}

diff --git a/src/main/java/ru/tehkode/permissions/backends/caching/CachingUserData.java b/src/main/java/ru/tehkode/permissions/backends/caching/CachingUserData.java
@@ -3,14 +3,13 @@
 import ru.tehkode.permissions.PermissionsUserData;
 
 import java.util.concurrent.Executor;
-import java.util.concurrent.locks.ReadWriteLock;
 
 /**
  * User data using a cache.
  */
 public class CachingUserData extends CachingData implements PermissionsUserData {
 	private final PermissionsUserData userData;
-	public CachingUserData(PermissionsUserData userData, Executor executor, ReadWriteLock lock) {
+	public CachingUserData(PermissionsUserData userData, Executor executor, Object lock) {
 		super(executor, lock);
 		this.userData = userData;
 	}
@@ -22,7 +21,7 @@ protected PermissionsUserData getBackingData() {
 
 	@Override
 	public boolean setIdentifier(final String identifier) {
-		executeWrite(new Runnable() {
+		execute(new Runnable() {
 			@Override
 			public void run() {
 				getBackingData().setIdentifier(identifier);