From 987ab7d2b6151a2f5eb053720f155412e9e5cc36 Mon Sep 17 00:00:00 2001 From: dordsor21 Date: Thu, 1 Oct 2020 14:33:34 +0100 Subject: [PATCH] reduce the amount of synchronisation/locks being used unneecessarily and add nullcheck --- .../BukkitPermissionAttachmentManager.java | 52 +++++++------------ .../sk89q/worldedit/bukkit/BukkitPlayer.java | 5 +- 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPermissionAttachmentManager.java b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPermissionAttachmentManager.java index 5c9461032..e3069c78e 100644 --- a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPermissionAttachmentManager.java +++ b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPermissionAttachmentManager.java @@ -1,58 +1,44 @@ package com.sk89q.worldedit.bukkit; +import javax.annotation.Nullable; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.locks.ReentrantReadWriteLock; import org.bukkit.entity.Player; import org.bukkit.permissions.PermissionAttachment; public class BukkitPermissionAttachmentManager { private final WorldEditPlugin plugin; - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - private final Map attachments = new HashMap(); + private final Map attachments = new HashMap<>(); public BukkitPermissionAttachmentManager(WorldEditPlugin plugin) { this.plugin = plugin; } - public synchronized PermissionAttachment addAttachment(Player p) { - PermissionAttachment attach; + public PermissionAttachment getOrAddAttachment(@Nullable final Player p) { + if (p == null) { + return null; + } + PermissionAttachment attachment = attachments.get(p); - //try find attachment - lock.readLock().lock(); - try { - attach = attachments.get(p); - } finally { - lock.readLock().unlock(); + if (attachment != null) { + return attachment; } - if (attach == null) { - lock.writeLock().lock(); - try { - //check again - do not remove this - attach = attachments.get(p); - if (attach == null) { - attach = p.addAttachment(plugin); - attachments.put(p, attach); - } - } finally { - lock.writeLock().unlock(); - } + synchronized (this) { + return attachments.computeIfAbsent(p, k -> k.addAttachment(plugin)); } - return attach; } - public void removeAttachment(Player p) { - PermissionAttachment attach; - lock.writeLock().lock(); - try { - attach = attachments.remove(p); - } finally { - lock.writeLock().unlock(); + public void removeAttachment(@Nullable final Player p) { + if (p == null || attachments.get(p) == null) { + return; } - if (attach != null) { - p.removeAttachment(attach); + synchronized (this) { + PermissionAttachment attach = attachments.remove(p); + if (attach != null) { + p.removeAttachment(attach); + } } } } diff --git a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java index e67a4f80d..be5cdd301 100644 --- a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java +++ b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java @@ -67,7 +67,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Locale; import java.util.UUID; -import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.annotation.Nullable; @@ -81,13 +80,13 @@ public class BukkitPlayer extends AbstractPlayerActor { super(getExistingMap(WorldEditPlugin.getInstance(), player)); this.plugin = WorldEditPlugin.getInstance(); this.player = player; - this.permAttachment = plugin.getPermissionAttachmentManager().addAttachment(player); + this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player); } public BukkitPlayer(WorldEditPlugin plugin, Player player) { this.plugin = plugin; this.player = player; - this.permAttachment = plugin.getPermissionAttachmentManager().addAttachment(player); + this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player); if (Settings.IMP.CLIPBOARD.USE_DISK) { loadClipboardFromDisk(); }