Remove the locks in NodeMap to ease thread contention when lots of processes are resolving inheritance & refactor the way LocalizedNodes are created (#734)

api/src/main/java/me/lucko/luckperms/api/Node.java

@@ -556,7 +556,7 @@ public interface Node {
          *
          * @param value the value
          * @return the builder
-         * @see Node#getValuePrimitive()
+         * @see Node#getValue()
          */
         @Nonnull
         Builder setValue(boolean value);

api/src/main/java/me/lucko/luckperms/api/StandardNodeEquality.java

@@ -44,7 +44,7 @@ public enum StandardNodeEquality implements NodeEqualityPredicate {
 
     /**
      * All attributes must match, except for
-     * {@link Node#getValuePrimitive() value}, which is ignored.
+     * {@link Node#getValue() value}, which is ignored.
      */
     IGNORE_VALUE,
 
@@ -59,14 +59,14 @@ public enum StandardNodeEquality implements NodeEqualityPredicate {
 
     /**
      * All attributes must match, except for
-     * {@link Node#getValuePrimitive() value} and the
+     * {@link Node#getValue() value} and the
      * {@link Node#getExpiry() expiry time}, which are ignored.
      */
     IGNORE_EXPIRY_TIME_AND_VALUE,
 
     /**
      * All attributes must match, except for
-     * {@link Node#getValuePrimitive() value} and the if the node is
+     * {@link Node#getValue() value} and the if the node is
      * {@link Node#isTemporary() temporary}, which are ignored.
      */
     IGNORE_VALUE_OR_IF_TEMPORARY;

common/src/main/java/me/lucko/luckperms/common/api/delegates/model/ApiPermissionHolder.java

@@ -46,6 +46,7 @@ import me.lucko.luckperms.common.model.NodeMapType;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.model.User;
 import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator;
+import me.lucko.luckperms.common.node.factory.NodeFactory;
 import me.lucko.luckperms.common.node.utils.MetaType;
 import me.lucko.luckperms.common.node.utils.NodeTools;
 import me.lucko.luckperms.common.utils.ImmutableCollectors;
@@ -104,13 +105,15 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol
     @Nonnull
     @Override
     public ImmutableSetMultimap<ImmutableContextSet, Node> getNodes() {
-        return this.handle.enduringData().immutable();
+        //noinspection unchecked
+        return (ImmutableSetMultimap) this.handle.enduringData().immutable();
     }
 
     @Nonnull
     @Override
     public ImmutableSetMultimap<ImmutableContextSet, Node> getTransientNodes() {
-        return this.handle.transientData().immutable();
+        //noinspection unchecked
+        return (ImmutableSetMultimap) this.handle.transientData().immutable();
     }
 
     @Nonnull
@@ -183,7 +186,7 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol
     @Override
     public Map<String, Boolean> exportNodes(@Nonnull Contexts contexts, boolean lowerCase) {
         Objects.requireNonNull(contexts, "contexts");
-        return ImmutableMap.copyOf(this.handle.exportNodesAndShorthand(contexts, lowerCase));
+        return ImmutableMap.copyOf(this.handle.exportPermissions(contexts, lowerCase, true));
     }
 
     @Nonnull
@@ -234,14 +237,26 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol
     @Override
     public boolean inheritsGroup(@Nonnull me.lucko.luckperms.api.Group group) {
         Objects.requireNonNull(group, "group");
-        return this.handle.inheritsGroup(ApiGroup.cast(group));
+
+        Group g = ApiGroup.cast(group);
+        if (this.handle.getType().isGroup() && g.getName().equals(this.handle.getObjectName())) {
+            return true;
+        }
+
+        return this.handle.hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(g.getName()).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean();
     }
 
     @Override
     public boolean inheritsGroup(@Nonnull me.lucko.luckperms.api.Group group, @Nonnull ContextSet contextSet) {
         Objects.requireNonNull(group, "group");
         Objects.requireNonNull(contextSet, "contextSet");
-        return this.handle.inheritsGroup(ApiGroup.cast(group), contextSet);
+
+        Group g = ApiGroup.cast(group);
+        if (this.handle.getType().isGroup() && g.getName().equals(this.handle.getObjectName())) {
+            return true;
+        }
+
+        return this.handle.hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(g.getName()).withExtraContext(contextSet).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean();
     }
 
     @Nonnull

common/src/main/java/me/lucko/luckperms/common/caching/AbstractCachedData.java

@@ -190,7 +190,7 @@ public abstract class AbstractCachedData implements CachedData {
         Objects.requireNonNull(contexts, "contexts");
 
         //noinspection ConstantConditions
-        return this.permission.get(contexts).join();
+        return this.permission.synchronous().get(contexts);
     }
 
     @Nonnull
@@ -199,7 +199,7 @@ public abstract class AbstractCachedData implements CachedData {
         Objects.requireNonNull(contexts, "contexts");
 
         //noinspection ConstantConditions
-        return this.meta.get(contexts).join();
+        return this.meta.synchronous().get(contexts);
     }
 
     @Nonnull

common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java

@@ -29,6 +29,7 @@ import me.lucko.luckperms.api.Contexts;
 import me.lucko.luckperms.api.caching.MetaContexts;
 import me.lucko.luckperms.common.caching.type.MetaAccumulator;
 import me.lucko.luckperms.common.calculators.CalculatorFactory;
+import me.lucko.luckperms.common.config.ConfigKeys;
 import me.lucko.luckperms.common.model.PermissionHolder;
 
 import java.util.Map;
@@ -60,12 +61,12 @@ public abstract class HolderCachedData<T extends PermissionHolder> extends Abstr
 
     @Override
     protected Map<String, Boolean> resolvePermissions() {
-        return this.holder.exportNodesAndShorthand(true);
+        return this.holder.exportPermissions(true, this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND));
     }
 
     @Override
     protected Map<String, Boolean> resolvePermissions(Contexts contexts) {
-        return this.holder.exportNodesAndShorthand(contexts, true);
+        return this.holder.exportPermissions(contexts, true, this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND));
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/commands/generic/parent/UserSwitchPrimaryGroup.java

@@ -25,6 +25,8 @@
 
 package me.lucko.luckperms.common.commands.generic.parent;
 
+import me.lucko.luckperms.api.Node;
+import me.lucko.luckperms.api.StandardNodeEquality;
 import me.lucko.luckperms.common.actionlog.ExtendedLogEntry;
 import me.lucko.luckperms.common.command.CommandResult;
 import me.lucko.luckperms.common.command.abstraction.SharedSubCommand;
@@ -36,6 +38,7 @@ import me.lucko.luckperms.common.locale.LocaleManager;
 import me.lucko.luckperms.common.locale.command.CommandSpec;
 import me.lucko.luckperms.common.locale.message.Message;
 import me.lucko.luckperms.common.model.Group;
+import me.lucko.luckperms.common.model.NodeMapType;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.model.User;
 import me.lucko.luckperms.common.node.factory.NodeFactory;
@@ -80,9 +83,10 @@ public class UserSwitchPrimaryGroup extends SharedSubCommand {
             return CommandResult.STATE_ERROR;
         }
 
-        if (!user.inheritsGroup(group)) {
+        Node node = NodeFactory.buildGroupNode(group.getName()).build();
+        if (!user.hasPermission(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_VALUE).asBoolean()) {
             Message.USER_PRIMARYGROUP_ERROR_NOTMEMBER.send(sender, user.getFriendlyName(), group.getName());
-            user.setPermission(NodeFactory.buildGroupNode(group.getName()).build());
+            user.setPermission(node);
         }
 
         user.getPrimaryGroup().setStoredValue(group.getName());

common/src/main/java/me/lucko/luckperms/common/commands/log/LogNotify.java

@@ -54,7 +54,7 @@ public class LogNotify extends SubCommand<Log> {
             return false;
         }
 
-        Optional<Node> ret = user.getOwnNodes().stream()
+        Optional<? extends Node> ret = user.getOwnNodes().stream()
                 .filter(n -> n.getPermission().equalsIgnoreCase("luckperms.log.notify.ignoring"))
                 .findFirst();
 

common/src/main/java/me/lucko/luckperms/common/event/EventFactory.java

@@ -170,17 +170,17 @@ public final class EventFactory {
         fireEventAsync(event);
     }
 
-    public void handleNodeAdd(Node node, PermissionHolder target, Collection<Node> before, Collection<Node> after) {
+    public void handleNodeAdd(Node node, PermissionHolder target, Collection<? extends Node> before, Collection<? extends Node> after) {
         EventNodeAdd event = new EventNodeAdd(node, getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after));
         fireEventAsync(event);
     }
 
-    public void handleNodeClear(PermissionHolder target, Collection<Node> before, Collection<Node> after) {
+    public void handleNodeClear(PermissionHolder target, Collection<? extends Node> before, Collection<? extends Node> after) {
         EventNodeClear event = new EventNodeClear(getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after));
         fireEventAsync(event);
     }
 
-    public void handleNodeRemove(Node node, PermissionHolder target, Collection<Node> before, Collection<Node> after) {
+    public void handleNodeRemove(Node node, PermissionHolder target, Collection<? extends Node> before, Collection<? extends Node> after) {
         EventNodeRemove event = new EventNodeRemove(node, getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after));
         fireEventAsync(event);
     }

common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java

@@ -82,7 +82,7 @@ public class InheritanceHandler {
         @Override
         public Iterable<? extends PermissionHolder> successors(PermissionHolder holder) {
             Set<Group> successors = new TreeSet<>(holder.getInheritanceComparator());
-            List<Node> nodes = holder.getOwnGroupNodes();
+            List<? extends Node> nodes = holder.getOwnGroupNodes();
             for (Node n : nodes) {
                 Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName());
                 if (g != null) {
@@ -109,7 +109,7 @@ public class InheritanceHandler {
         @Override
         public Iterable<? extends PermissionHolder> successors(PermissionHolder holder) {
             Set<Group> successors = new TreeSet<>(holder.getInheritanceComparator());
-            List<Node> nodes = holder.getOwnGroupNodes(this.context.getContexts());
+            List<? extends Node> nodes = holder.getOwnGroupNodes(this.context.getContexts());
             for (Node n : nodes) {
                 // effectively: if not (we're applying global groups or it's specific anyways)
                 if (!((this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_SERVER) || n.isServerSpecific()) && (this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_WORLD) || n.isWorldSpecific()))) {

common/src/main/java/me/lucko/luckperms/common/model/NodeMap.java

@@ -25,9 +25,10 @@
 
 package me.lucko.luckperms.common.model;
 
+import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.Multimaps;
 import com.google.common.collect.SortedSetMultimap;
 
 import me.lucko.luckperms.api.LocalizedNode;
@@ -50,7 +51,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
@@ -66,6 +68,8 @@ import javax.annotation.Nullable;
  * <p>Each holder has two of these maps, one for enduring and transient nodes.</p>
  */
 public final class NodeMap {
+    @SuppressWarnings("Guava")
+    private static final Supplier<SortedSet<LocalizedNode>> VALUE_SET_SUPPLIER = () -> new ConcurrentSkipListSet<>(NodeComparator.reverse());
 
     /**
      * The holder which this map is for
@@ -80,24 +84,19 @@ public final class NodeMap {
      * key, and finally by the overall size of the set. Nodes are ordered according to the priority rules
      * defined in {@link NodeComparator}.</p>
      */
-    private final SortedSetMultimap<ImmutableContextSet, Node> map = MultimapBuilder
+    private final SortedSetMultimap<ImmutableContextSet, LocalizedNode> map = Multimaps.newSortedSetMultimap(
-            .treeKeys(ContextSetComparator.reverse())
+            new ConcurrentSkipListMap<>(ContextSetComparator.reverse()),
-            .treeSetValues(NodeComparator.reverse())
+            VALUE_SET_SUPPLIER
-            .build();
+    );
 
     /**
      * Copy of {@link #map} which only contains group nodes
      * @see Node#isGroupNode()
      */
-    private final SortedSetMultimap<ImmutableContextSet, Node> inheritanceMap = MultimapBuilder
+    private final SortedSetMultimap<ImmutableContextSet, LocalizedNode> inheritanceMap = Multimaps.newSortedSetMultimap(
-            .treeKeys(ContextSetComparator.reverse())
+            new ConcurrentSkipListMap<>(ContextSetComparator.reverse()),
-            .treeSetValues(NodeComparator.reverse())
+            VALUE_SET_SUPPLIER
-            .build();
+    );
-
-    /**
-     * The lock which synchronizes the instance
-     */
-    private final ReentrantLock lock = new ReentrantLock();
 
     /**
      * A cache which holds an immutable copy of the backing map
@@ -108,82 +107,41 @@ public final class NodeMap {
         this.holder = holder;
     }
 
-    public List<Node> asList() {
+    public List<LocalizedNode> asList() {
-        this.lock.lock();
+        return new ArrayList<>(this.map.values());
-        try {
-            return new ArrayList<>(this.map.values());
-        } finally {
-            this.lock.unlock();
-        }
     }
 
-    public LinkedHashSet<Node> asSet() {
+    public LinkedHashSet<LocalizedNode> asSet() {
-        this.lock.lock();
+        return new LinkedHashSet<>(this.map.values());
-        try {
-            return new LinkedHashSet<>(this.map.values());
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     public SortedSet<LocalizedNode> asSortedSet() {
         SortedSet<LocalizedNode> ret = new TreeSet<>(NodeWithContextComparator.reverse());
-        copyToLocalized(ret);
+        copyTo(ret);
         return ret;
     }
 
-    public void copyTo(Collection<? super Node> collection) {
+    public void copyTo(Collection<? super LocalizedNode> collection) {
-        this.lock.lock();
+        collection.addAll(this.map.values());
-        try {
-            collection.addAll(this.map.values());
-        } finally {
-            this.lock.unlock();
-        }
     }
 
-    public void copyTo(Collection<? super Node> collection, ContextSet filter) {
+    public void copyTo(Collection<? super LocalizedNode> collection, ContextSet filter) {
-        this.lock.lock();
+        for (Map.Entry<ImmutableContextSet, Collection<LocalizedNode>> e : this.map.asMap().entrySet()) {
-        try {
+            if (e.getKey().isSatisfiedBy(filter)) {
-            for (Map.Entry<ImmutableContextSet, Collection<Node>> e : this.map.asMap().entrySet()) {
+                collection.addAll(e.getValue());
-                if (e.getKey().isSatisfiedBy(filter)) {
-                    collection.addAll(e.getValue());
-                }
             }
-        } finally {
-            this.lock.unlock();
         }
     }
 
-    public void copyGroupNodesTo(Collection<? super Node> collection) {
+    public void copyGroupNodesTo(Collection<? super LocalizedNode> collection) {
-        this.lock.lock();
+        collection.addAll(this.inheritanceMap.values());
-        try {
-            collection.addAll(this.inheritanceMap.values());
-        } finally {
-            this.lock.unlock();
-        }
     }
 
-    public void copyGroupNodesTo(Collection<? super Node> collection, ContextSet filter) {
+    public void copyGroupNodesTo(Collection<? super LocalizedNode> collection, ContextSet filter) {
-        this.lock.lock();
+        for (Map.Entry<ImmutableContextSet, Collection<LocalizedNode>> e : this.inheritanceMap.asMap().entrySet()) {
-        try {
+            if (e.getKey().isSatisfiedBy(filter)) {
-            for (Map.Entry<ImmutableContextSet, Collection<Node>> e : this.inheritanceMap.asMap().entrySet()) {
+                collection.addAll(e.getValue());
-                if (e.getKey().isSatisfiedBy(filter)) {
-                    collection.addAll(e.getValue());
-                }
             }
-        } finally {
-            this.lock.unlock();
-        }
-    }
-
-    public void copyToLocalized(Collection<LocalizedNode> collection) {
-        this.lock.lock();
-        try {
-            for (Node node : this.map.values()) {
-                collection.add(ImmutableLocalizedNode.of(node, this.holder.getObjectName()));
-            }
-        } finally {
-            this.lock.unlock();
         }
     }
 
@@ -192,7 +150,7 @@ public final class NodeMap {
      *
      * @return an immutable copy
      */
-    public ImmutableSetMultimap<ImmutableContextSet, Node> immutable() {
+    public ImmutableSetMultimap<ImmutableContextSet, LocalizedNode> immutable() {
         return this.cache.get();
     }
 
@@ -203,161 +161,108 @@ public final class NodeMap {
         this.cache.invalidate();
     }
 
-    void add(Node node) {
+    private LocalizedNode localise(Node node) {
-        this.lock.lock();
+        if (node instanceof LocalizedNode) {
-        try {
+            LocalizedNode localizedNode = (LocalizedNode) node;
-            ImmutableContextSet context = node.getFullContexts().makeImmutable();
+            if (this.holder.getObjectName().equals(localizedNode.getLocation())) {
-            this.map.put(context, node);
+                return localizedNode;
-            if (node.isGroupNode() && node.getValue()) {
-                this.inheritanceMap.put(context, node);
             }
-        } finally {
+        }
-            this.lock.unlock();
+
+        // localise
+        return ImmutableLocalizedNode.of(node, this.holder.getObjectName());
+    }
+
+    void add(Node node) {
+        ImmutableContextSet context = node.getFullContexts().makeImmutable();
+        LocalizedNode n = localise(node);
+
+        this.map.put(context, n);
+        if (node.isGroupNode() && node.getValue()) {
+            this.inheritanceMap.put(context, n);
         }
     }
 
     void remove(Node node) {
-        this.lock.lock();
+        ImmutableContextSet context = node.getFullContexts().makeImmutable();
-        try {
+        this.map.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE));
-            ImmutableContextSet context = node.getFullContexts().makeImmutable();
+        if (node.isGroupNode()) {
-            this.map.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE));
+            this.inheritanceMap.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE));
-            if (node.isGroupNode()) {
-                this.inheritanceMap.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE));
-            }
-        } finally {
-            this.lock.unlock();
         }
     }
 
     private void removeExact(Node node) {
-        this.lock.lock();
+        ImmutableContextSet context = node.getFullContexts().makeImmutable();
-        try {
+        this.map.remove(context, node);
-            ImmutableContextSet context = node.getFullContexts().makeImmutable();
+        if (node.isGroupNode() && node.getValue()) {
-            this.map.remove(context, node);
+            this.inheritanceMap.remove(context, node);
-            if (node.isGroupNode() && node.getValue()) {
-                this.inheritanceMap.remove(context, node);
-            }
-        } finally {
-            this.lock.unlock();
         }
     }
 
     void replace(Node node, Node previous) {
-        this.lock.lock();
+        removeExact(previous);
-        try {
+        add(node);
-            removeExact(previous);
-            add(node);
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     void clear() {
-        this.lock.lock();
+        this.map.clear();
-        try {
+        this.inheritanceMap.clear();
-            this.map.clear();
-            this.inheritanceMap.clear();
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     void clear(ContextSet contextSet) {
-        this.lock.lock();
+        ImmutableContextSet context = contextSet.makeImmutable();
-        try {
+        this.map.removeAll(context);
-            ImmutableContextSet context = contextSet.makeImmutable();
+        this.inheritanceMap.removeAll(context);
-            this.map.removeAll(context);
+    }
-            this.inheritanceMap.removeAll(context);
+
-        } finally {
+    void setContent(Collection<? extends Node> set) {
-            this.lock.unlock();
+        this.map.clear();
+        this.inheritanceMap.clear();
+        for (Node n : set) {
+            add(n);
         }
     }
 
-    void setContent(Set<Node> set) {
+    void setContent(Multimap<ImmutableContextSet, ? extends Node> multimap) {
-        this.lock.lock();
+        setContent(multimap.values());
-        try {
-            this.map.clear();
-            this.inheritanceMap.clear();
-            for (Node n : set) {
-                add(n);
-            }
-        } finally {
-            this.lock.unlock();
-        }
-    }
-
-    void setContent(Multimap<ImmutableContextSet, Node> multimap) {
-        this.lock.lock();
-        try {
-            this.map.clear();
-            this.inheritanceMap.clear();
-
-            this.map.putAll(multimap);
-            for (Map.Entry<ImmutableContextSet, Node> entry : this.map.entries()) {
-                if (entry.getValue().isGroupNode() && entry.getValue().getValue()) {
-                    this.inheritanceMap.put(entry.getKey(), entry.getValue());
-                }
-            }
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     boolean removeIf(Predicate<? super Node> predicate) {
-        this.lock.lock();
+        boolean ret = this.map.values().removeIf(predicate);
-        try {
+        this.inheritanceMap.values().removeIf(predicate);
-            boolean ret = this.map.values().removeIf(predicate);
+        return ret;
-            if (ret) {
-                this.inheritanceMap.values().removeIf(predicate);
-            }
-            return ret;
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     boolean removeIf(ContextSet contextSet, Predicate<? super Node> predicate) {
-        this.lock.lock();
+        ImmutableContextSet context = contextSet.makeImmutable();
-        try {
+        SortedSet<LocalizedNode> nodes = this.map.get(context);
-            ImmutableContextSet context = contextSet.makeImmutable();
+        boolean ret = nodes.removeIf(predicate);
-            SortedSet<Node> nodes = this.map.get(context);
+        this.inheritanceMap.get(context).removeIf(predicate);
-            boolean ret = nodes.removeIf(predicate);
+        return ret;
-            if (ret) {
-                this.inheritanceMap.get(context).removeIf(predicate);
-            }
-            return ret;
-        } finally {
-            this.lock.unlock();
-        }
     }
 
     boolean auditTemporaryNodes(@Nullable Set<Node> removed) {
         boolean work = false;
 
-        this.lock.lock();
+        Iterator<? extends Node> it = this.map.values().iterator();
-        try {
+        while (it.hasNext()) {
-            Iterator<Node> it = this.map.values().iterator();
+            Node entry = it.next();
-            while (it.hasNext()) {
+            if (entry.hasExpired()) {
-                Node entry = it.next();
+                if (removed != null) {
-                if (entry.hasExpired()) {
+                    removed.add(entry);
-                    if (removed != null) {
-                        removed.add(entry);
-                    }
-                    if (entry.isGroupNode() && entry.getValue()) {
-                        this.inheritanceMap.remove(entry.getFullContexts().makeImmutable(), entry);
-                    }
-                    work = true;
-                    it.remove();
                 }
+                if (entry.isGroupNode() && entry.getValue()) {
+                    this.inheritanceMap.remove(entry.getFullContexts().makeImmutable(), entry);
+                }
+                work = true;
+                it.remove();
             }
-        } finally {
-            this.lock.unlock();
         }
 
         return work;
     }
 
-    private static final class NodeMapCache extends Cache<ImmutableSetMultimap<ImmutableContextSet, Node>> {
+    private static final class NodeMapCache extends Cache<ImmutableSetMultimap<ImmutableContextSet, LocalizedNode>> {
         private final NodeMap handle;
 
         private NodeMapCache(NodeMap handle) {
@@ -366,13 +271,8 @@ public final class NodeMap {
 
         @Nonnull
         @Override
-        protected ImmutableSetMultimap<ImmutableContextSet, Node> supply() {
+        protected ImmutableSetMultimap<ImmutableContextSet, LocalizedNode> supply() {
-            this.handle.lock.lock();
+            return ImmutableSetMultimap.copyOf(this.handle.map);
-            try {
-                return ImmutableSetMultimap.copyOf(this.handle.map);
-            } finally {
-                this.handle.lock.unlock();
-            }
         }
     }
 

common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java

@@ -46,8 +46,6 @@ import me.lucko.luckperms.common.config.ConfigKeys;
 import me.lucko.luckperms.common.inheritance.InheritanceComparator;
 import me.lucko.luckperms.common.inheritance.InheritanceGraph;
 import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator;
-import me.lucko.luckperms.common.node.factory.NodeFactory;
-import me.lucko.luckperms.common.node.model.ImmutableLocalizedNode;
 import me.lucko.luckperms.common.node.utils.InheritanceInfo;
 import me.lucko.luckperms.common.node.utils.MetaType;
 import me.lucko.luckperms.common.node.utils.NodeTools;
@@ -218,39 +216,39 @@ public abstract class PermissionHolder {
         reloadCachedData();
     }
 
-    public void setNodes(NodeMapType type, Set<Node> set) {
+    public void setNodes(NodeMapType type, Set<? extends Node> set) {
         getData(type).setContent(set);
         invalidateCache();
     }
 
-    public void replaceNodes(NodeMapType type, Multimap<ImmutableContextSet, Node> multimap) {
+    public void replaceNodes(NodeMapType type, Multimap<ImmutableContextSet, ? extends Node> multimap) {
         getData(type).setContent(multimap);
         invalidateCache();
     }
 
-    public List<Node> getOwnNodes() {
+    public List<LocalizedNode> getOwnNodes() {
-        List<Node> ret = new ArrayList<>();
+        List<LocalizedNode> ret = new ArrayList<>();
         this.transientNodes.copyTo(ret);
         this.enduringNodes.copyTo(ret);
         return ret;
     }
 
-    public List<Node> getOwnNodes(ContextSet filter) {
+    public List<LocalizedNode> getOwnNodes(ContextSet filter) {
-        List<Node> ret = new ArrayList<>();
+        List<LocalizedNode> ret = new ArrayList<>();
         this.transientNodes.copyTo(ret, filter);
         this.enduringNodes.copyTo(ret, filter);
         return ret;
     }
 
-    public List<Node> getOwnGroupNodes() {
+    public List<LocalizedNode> getOwnGroupNodes() {
-        List<Node> ret = new ArrayList<>();
+        List<LocalizedNode> ret = new ArrayList<>();
         this.transientNodes.copyGroupNodesTo(ret);
         this.enduringNodes.copyGroupNodesTo(ret);
         return ret;
     }
 
-    public List<Node> getOwnGroupNodes(ContextSet filter) {
+    public List<LocalizedNode> getOwnGroupNodes(ContextSet filter) {
-        List<Node> ret = new ArrayList<>();
+        List<LocalizedNode> ret = new ArrayList<>();
         this.transientNodes.copyGroupNodesTo(ret, filter);
         this.enduringNodes.copyGroupNodesTo(ret, filter);
         return ret;
@@ -258,17 +256,17 @@ public abstract class PermissionHolder {
 
     public SortedSet<LocalizedNode> getOwnNodesSorted() {
         SortedSet<LocalizedNode> ret = new TreeSet<>(NodeWithContextComparator.reverse());
-        this.transientNodes.copyToLocalized(ret);
+        this.transientNodes.copyTo(ret);
-        this.enduringNodes.copyToLocalized(ret);
+        this.enduringNodes.copyTo(ret);
         return ret;
     }
 
-    public boolean removeIf(Predicate<Node> predicate) {
+    public boolean removeIf(Predicate<? super Node> predicate) {
         return removeIf(predicate, null);
     }
 
-    public boolean removeIf(Predicate<Node> predicate, Runnable taskIfSuccess) {
+    public boolean removeIf(Predicate<? super Node> predicate, Runnable taskIfSuccess) {
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         if (!this.enduringNodes.removeIf(predicate)) {
             return false;
         }
@@ -276,18 +274,18 @@ public abstract class PermissionHolder {
             taskIfSuccess.run();
         }
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         this.plugin.getEventFactory().handleNodeClear(this, before, after);
         return true;
     }
 
-    public boolean removeIf(ContextSet contextSet, Predicate<Node> predicate) {
+    public boolean removeIf(ContextSet contextSet, Predicate<? super Node> predicate) {
         return removeIf(contextSet, predicate, null);
     }
 
-    public boolean removeIf(ContextSet contextSet, Predicate<Node> predicate, Runnable taskIfSuccess) {
+    public boolean removeIf(ContextSet contextSet, Predicate<? super Node> predicate, Runnable taskIfSuccess) {
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         if (!this.enduringNodes.removeIf(contextSet, predicate)) {
             return false;
         }
@@ -295,13 +293,13 @@ public abstract class PermissionHolder {
             taskIfSuccess.run();
         }
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         this.plugin.getEventFactory().handleNodeClear(this, before, after);
         return true;
     }
 
-    public boolean removeIfTransient(Predicate<Node> predicate) {
+    public boolean removeIfTransient(Predicate<? super Node> predicate) {
         boolean result = this.transientNodes.removeIf(predicate);
         if (result) {
             invalidateCache();
@@ -309,15 +307,12 @@ public abstract class PermissionHolder {
         return result;
     }
 
-    public void accumulateInheritancesTo(List<LocalizedNode> accumulator, Contexts context) {
+    public void accumulateInheritancesTo(List<? super LocalizedNode> accumulator, Contexts context) {
         InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context);
         Iterable<PermissionHolder> traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this);
         for (PermissionHolder holder : traversal) {
-            List<Node> nodes = holder.getOwnNodes(context.getContexts());
+            List<? extends LocalizedNode> nodes = holder.getOwnNodes(context.getContexts());
-            for (Node node : nodes) {
+            accumulator.addAll(nodes);
-                ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(node, holder.getObjectName());
-                accumulator.add(localizedNode);
-            }
         }
     }
 
@@ -327,15 +322,12 @@ public abstract class PermissionHolder {
         return accumulator;
     }
 
-    public void accumulateInheritancesTo(List<LocalizedNode> accumulator) {
+    public void accumulateInheritancesTo(List<? super LocalizedNode> accumulator) {
         InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph();
         Iterable<PermissionHolder> traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this);
         for (PermissionHolder holder : traversal) {
-            List<Node> nodes = holder.getOwnNodes();
+            List<? extends LocalizedNode> nodes = holder.getOwnNodes();
-            for (Node node : nodes) {
+            accumulator.addAll(nodes);
-                ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(node, holder.getObjectName());
-                accumulator.add(localizedNode);
-            }
         }
     }
 
@@ -350,10 +342,7 @@ public abstract class PermissionHolder {
         if (context.hasSetting(LookupSetting.RESOLVE_INHERITANCE)) {
             accumulateInheritancesTo(entries, context);
         } else {
-            for (Node n : getOwnNodes(context.getContexts())) {
+            entries.addAll(getOwnNodes(context.getContexts()));
-                ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(n, getObjectName());
-                entries.add(localizedNode);
-            }
         }
 
         if (!context.hasSetting(LookupSetting.INCLUDE_NODES_SET_WITHOUT_SERVER)) {
@@ -366,42 +355,34 @@ public abstract class PermissionHolder {
         return entries;
     }
 
-    public Map<String, Boolean> exportNodesAndShorthand(Contexts context, boolean lowerCase) {
+    public Map<String, Boolean> exportPermissions(Contexts context, boolean convertToLowercase, boolean resolveShorthand) {
         List<LocalizedNode> entries = getAllEntries(context);
+        return processExportedPermissions(entries, convertToLowercase, resolveShorthand);
+    }
 
-        Map<String, Boolean> perms = new HashMap<>();
+    public Map<String, Boolean> exportPermissions(boolean convertToLowercase, boolean resolveShorthand) {
-        boolean applyShorthand = this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND);
+        List<LocalizedNode> entries = resolveInheritances();
+        return processExportedPermissions(entries, convertToLowercase, resolveShorthand);
+    }
+
+    private static ImmutableMap<String, Boolean> processExportedPermissions(List<LocalizedNode> entries, boolean convertToLowercase, boolean resolveShorthand) {
+        Map<String, Boolean> perms = new HashMap<>(entries.size());
         for (Node node : entries) {
-            String perm = lowerCase ? node.getPermission().toLowerCase() : node.getPermission();
+            if (convertToLowercase) {
-
+                perms.putIfAbsent(node.getPermission().toLowerCase(), node.getValue());
-            if (perms.putIfAbsent(perm, node.getValue()) == null) {
+            } else {
-                if (applyShorthand) {
+                perms.putIfAbsent(node.getPermission(), node.getValue());
-                    List<String> shorthand = node.resolveShorthand();
-                    if (!shorthand.isEmpty()) {
-                        for (String s : shorthand) {
-                            perms.putIfAbsent(lowerCase ? s.toLowerCase() : s, node.getValue());
-                        }
-                    }
-                }
             }
         }
 
-        return ImmutableMap.copyOf(perms);
+        if (resolveShorthand) {
-    }
+            for (Node node : entries) {
-
-    public Map<String, Boolean> exportNodesAndShorthand(boolean lowerCase) {
-        List<LocalizedNode> entries = resolveInheritances();
-
-        Map<String, Boolean> perms = new HashMap<>();
-        boolean applyShorthand = this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND);
-        for (Node node : entries) {
-            String perm = lowerCase ? node.getPermission().toLowerCase().intern() : node.getPermission();
-
-            if (perms.putIfAbsent(perm, node.getValue()) == null && applyShorthand) {
                 List<String> shorthand = node.resolveShorthand();
-                if (!shorthand.isEmpty()) {
+                for (String s : shorthand) {
-                    for (String s : shorthand) {
+                    if (convertToLowercase) {
-                        perms.putIfAbsent((lowerCase ? s.toLowerCase() : s).intern(), node.getValue());
+                        perms.putIfAbsent(s.toLowerCase(), node.getValue());
+                    } else {
+                        perms.putIfAbsent(s, node.getValue());
                     }
                 }
             }
@@ -418,8 +399,8 @@ public abstract class PermissionHolder {
         InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context);
         Iterable<PermissionHolder> traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this);
         for (PermissionHolder holder : traversal) {
-            List<Node> nodes = holder.getOwnNodes(context.getContexts());
+            List<? extends LocalizedNode> nodes = holder.getOwnNodes(context.getContexts());
-            for (Node node : nodes) {
+            for (LocalizedNode node : nodes) {
                 if (!node.getValue()) continue;
                 if (!node.isMeta() && !node.isPrefix() && !node.isSuffix()) continue;
 
@@ -427,7 +408,7 @@ public abstract class PermissionHolder {
                     continue;
                 }
 
-                accumulator.accumulateNode(ImmutableLocalizedNode.of(node, holder.getObjectName()));
+                accumulator.accumulateNode(node);
             }
 
             OptionalInt w = holder.getWeight();
@@ -447,12 +428,12 @@ public abstract class PermissionHolder {
         InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph();
         Iterable<PermissionHolder> traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this);
         for (PermissionHolder holder : traversal) {
-            List<Node> nodes = holder.getOwnNodes();
+            List<? extends LocalizedNode> nodes = holder.getOwnNodes();
-            for (Node node : nodes) {
+            for (LocalizedNode node : nodes) {
                 if (!node.getValue()) continue;
                 if (!node.isMeta() && !node.isPrefix() && !node.isSuffix()) continue;
 
-                accumulator.accumulateNode(ImmutableLocalizedNode.of(node, holder.getObjectName()));
+                accumulator.accumulateNode(node);
             }
 
             OptionalInt w = getWeight();
@@ -474,7 +455,7 @@ public abstract class PermissionHolder {
         // we don't call events for transient nodes
         boolean transientWork = this.transientNodes.auditTemporaryNodes(null);
 
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         Set<Node> removed = new HashSet<>();
 
         boolean enduringWork = this.enduringNodes.auditTemporaryNodes(removed);
@@ -483,7 +464,7 @@ public abstract class PermissionHolder {
             invalidateCache();
 
             // call event
-            ImmutableCollection<Node> after = enduringData().immutable().values();
+            ImmutableCollection<? extends Node> after = enduringData().immutable().values();
             for (Node r : removed) {
                 this.plugin.getEventFactory().handleNodeRemove(r, this, before, after);
             }
@@ -496,8 +477,8 @@ public abstract class PermissionHolder {
         return transientWork || enduringWork;
     }
 
-    private Optional<Node> searchForMatch(NodeMapType type, Node node, NodeEqualityPredicate equalityPredicate) {
+    private Optional<LocalizedNode> searchForMatch(NodeMapType type, Node node, NodeEqualityPredicate equalityPredicate) {
-        for (Node n : getData(type).immutable().values()) {
+        for (LocalizedNode n : getData(type).immutable().values()) {
             if (n.equals(node, equalityPredicate)) {
                 return Optional.of(n);
             }
@@ -559,10 +540,10 @@ public abstract class PermissionHolder {
             return DataMutateResult.ALREADY_HAS;
         }
 
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         this.enduringNodes.add(node);
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         this.plugin.getEventFactory().handleNodeAdd(node, this, before, after);
         return DataMutateResult.SUCCESS;
@@ -579,7 +560,7 @@ public abstract class PermissionHolder {
         if (node.isTemporary()) {
             if (modifier == TemporaryModifier.ACCUMULATE) {
                 // Try to accumulate with an existing node
-                Optional<Node> existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE);
+                Optional<? extends Node> existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE);
 
                 // An existing node was found
                 if (existing.isPresent()) {
@@ -589,10 +570,10 @@ public abstract class PermissionHolder {
                     Node newNode = node.toBuilder().setExpiry(previous.getExpiryUnixTime() + node.getSecondsTilExpiry()).build();
 
                     // Remove the old node & add the new one.
-                    ImmutableCollection<Node> before = enduringData().immutable().values();
+                    ImmutableCollection<? extends Node> before = enduringData().immutable().values();
                     this.enduringNodes.replace(newNode, previous);
                     invalidateCache();
-                    ImmutableCollection<Node> after = enduringData().immutable().values();
+                    ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
                     this.plugin.getEventFactory().handleNodeAdd(newNode, this, before, after);
                     return Maps.immutableEntry(DataMutateResult.SUCCESS, newNode);
@@ -600,7 +581,7 @@ public abstract class PermissionHolder {
 
             } else if (modifier == TemporaryModifier.REPLACE) {
                 // Try to replace an existing node
-                Optional<Node> existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE);
+                Optional<? extends Node> existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE);
 
                 // An existing node was found
                 if (existing.isPresent()) {
@@ -609,10 +590,10 @@ public abstract class PermissionHolder {
                     // Only replace if the new expiry time is greater than the old one.
                     if (node.getExpiryUnixTime() > previous.getExpiryUnixTime()) {
 
-                        ImmutableCollection<Node> before = enduringData().immutable().values();
+                        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
                         this.enduringNodes.replace(node, previous);
                         invalidateCache();
-                        ImmutableCollection<Node> after = enduringData().immutable().values();
+                        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
                         this.plugin.getEventFactory().handleNodeAdd(node, this, before, after);
                         return Maps.immutableEntry(DataMutateResult.SUCCESS, node);
@@ -652,10 +633,10 @@ public abstract class PermissionHolder {
             return DataMutateResult.LACKS;
         }
 
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         this.enduringNodes.remove(node);
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         this.plugin.getEventFactory().handleNodeRemove(node, this, before, after);
         return DataMutateResult.SUCCESS;
@@ -676,22 +657,14 @@ public abstract class PermissionHolder {
         return DataMutateResult.SUCCESS;
     }
 
-    public boolean inheritsGroup(Group group) {
-        return group.getName().equalsIgnoreCase(this.getObjectName()) || hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(group.getName()).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean();
-    }
-
-    public boolean inheritsGroup(Group group, ContextSet contextSet) {
-        return group.getName().equalsIgnoreCase(this.getObjectName()) || hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(group.getName()).withExtraContext(contextSet).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean();
-    }
-
     /**
      * Clear all of the holders permission nodes
      */
     public boolean clearNodes() {
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         this.enduringNodes.clear();
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         if (before.size() == after.size()) {
             return false;
@@ -702,10 +675,10 @@ public abstract class PermissionHolder {
     }
 
     public boolean clearNodes(ContextSet contextSet) {
-        ImmutableCollection<Node> before = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> before = enduringData().immutable().values();
         this.enduringNodes.clear(contextSet);
         invalidateCache();
-        ImmutableCollection<Node> after = enduringData().immutable().values();
+        ImmutableCollection<? extends Node> after = enduringData().immutable().values();
 
         if (before.size() == after.size()) {
             return false;

common/src/main/java/me/lucko/luckperms/common/node/model/ImmutableLocalizedNode.java

@@ -39,6 +39,12 @@ public final class ImmutableLocalizedNode extends ForwardingNode implements Loca
     public static ImmutableLocalizedNode of(Node node, String location) {
         Objects.requireNonNull(node, "node");
         Objects.requireNonNull(location, "location");
+
+        // unwrap
+        while (node instanceof LocalizedNode) {
+            node = ((LocalizedNode) node).getNode();
+        }
+
         return new ImmutableLocalizedNode(node, location);
     }
 

sponge/src/main/java/me/lucko/luckperms/sponge/service/internal/HolderSubjectData.java

@@ -72,7 +72,7 @@ public class HolderSubjectData implements LPSubjectData {
         this.parentSubject = parentSubject;
     }
 
-    private Stream<Node> streamNodes() {
+    private Stream<? extends Node> streamNodes() {
         return this.holder.getData(this.type).immutable().values().stream();
     }
 
@@ -94,7 +94,7 @@ public class HolderSubjectData implements LPSubjectData {
     @Override
     public ImmutableMap<ImmutableContextSet, ImmutableMap<String, Boolean>> getAllPermissions() {
         ImmutableMap.Builder<ImmutableContextSet, ImmutableMap<String, Boolean>> ret = ImmutableMap.builder();
-        for (Map.Entry<ImmutableContextSet, Collection<Node>> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) {
+        for (Map.Entry<ImmutableContextSet, ? extends Collection<? extends Node>> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) {
             ImmutableMap.Builder<String, Boolean> builder = ImmutableMap.builder();
             for (Node n : entry.getValue()) {
                 builder.put(n.getPermission(), n.getValue());
@@ -183,7 +183,7 @@ public class HolderSubjectData implements LPSubjectData {
     @Override
     public ImmutableMap<ImmutableContextSet, ImmutableList<LPSubjectReference>> getAllParents() {
         ImmutableMap.Builder<ImmutableContextSet, ImmutableList<LPSubjectReference>> ret = ImmutableMap.builder();
-        for (Map.Entry<ImmutableContextSet, Collection<Node>> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) {
+        for (Map.Entry<ImmutableContextSet, ? extends Collection<? extends Node>> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) {
             ImmutableList.Builder<LPSubjectReference> builder = ImmutableList.builder();
             for (Node n : entry.getValue()) {
                 if (n.isGroupNode()) {