1
0
mirror of https://github.com/LuckPerms/LuckPerms.git synced 2025-01-23 16:51:43 +01:00

Tidy up node + context comparators a bit

This commit is contained in:
Luck 2021-01-14 20:45:51 +00:00
parent be1b9d45fa
commit 73230bc9b6
No known key found for this signature in database
GPG Key ID: EFA9B3EC5FD90F8B
6 changed files with 88 additions and 80 deletions

View File

@ -25,14 +25,14 @@
package me.lucko.luckperms.common.context; package me.lucko.luckperms.common.context;
import me.lucko.luckperms.common.context.contextset.ImmutableContextSetImpl;
import net.luckperms.api.context.Context; import net.luckperms.api.context.Context;
import net.luckperms.api.context.DefaultContextKeys; import net.luckperms.api.context.DefaultContextKeys;
import net.luckperms.api.context.ImmutableContextSet; import net.luckperms.api.context.ImmutableContextSet;
import java.util.ArrayList; import java.util.Arrays;
import java.util.Comparator; import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
public class ContextSetComparator implements Comparator<ImmutableContextSet> { public class ContextSetComparator implements Comparator<ImmutableContextSet> {
@ -53,40 +53,41 @@ public class ContextSetComparator implements Comparator<ImmutableContextSet> {
return 0; return 0;
} }
// compare presence of a server context
int result = Boolean.compare(o1.containsKey(DefaultContextKeys.SERVER_KEY), o2.containsKey(DefaultContextKeys.SERVER_KEY)); int result = Boolean.compare(o1.containsKey(DefaultContextKeys.SERVER_KEY), o2.containsKey(DefaultContextKeys.SERVER_KEY));
if (result != 0) { if (result != 0) {
return result; return result;
} }
// compare presence of a world context
result = Boolean.compare(o1.containsKey(DefaultContextKeys.WORLD_KEY), o2.containsKey(DefaultContextKeys.WORLD_KEY)); result = Boolean.compare(o1.containsKey(DefaultContextKeys.WORLD_KEY), o2.containsKey(DefaultContextKeys.WORLD_KEY));
if (result != 0) { if (result != 0) {
return result; return result;
} }
// compare overall size
result = Integer.compare(o1.size(), o2.size()); result = Integer.compare(o1.size(), o2.size());
if (result != 0) { if (result != 0) {
return result; return result;
} }
// we *have* to maintain transitivity in this comparator. this may be expensive, but it's necessary, as this // At this point, we don't really care about the order between the two sets.
// comparator is used in the PermissionHolder nodes treemap // However, we *have* to maintain transitivity in this comparator (despite how
// expensive/complex it may be) as it is used in the PermissionHolder nodes treemap.
// in order to have consistent ordering, we have to compare the content of the context sets by ordering the // in order to have consistent ordering, we have to compare the content of the context sets
// elements and then comparing which set is greater. // by sorting the contents and then comparing which set is greater.
List<Context> o1Entries = new ArrayList<>(o1.toSet()); Context[] o1Array = o1 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o1).toArray() : o1.toSet().toArray(new Context[0]);
List<Context> o2Entries = new ArrayList<>(o2.toSet()); Context[] o2Array = o2 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o2).toArray() : o2.toSet().toArray(new Context[0]);
o1Entries.sort(CONTEXT_COMPARATOR);
o2Entries.sort(CONTEXT_COMPARATOR);
// size is definitely the same Arrays.sort(o1Array, CONTEXT_COMPARATOR);
Iterator<Context> it1 = o1Entries.iterator(); Arrays.sort(o2Array, CONTEXT_COMPARATOR);
Iterator<Context> it2 = o2Entries.iterator();
while (it1.hasNext()) { for (int i = 0; i < o1Array.length; i++) {
Context ent1 = it1.next(); Context ent1 = o1Array[i];
Context ent2 = it2.next(); Context ent2 = o2Array[i];
result = CONTEXT_COMPARATOR.compare(ent1, ent2); result = compareContexts(ent1, ent2);
if (result != 0) { if (result != 0) {
return result; return result;
} }
@ -95,19 +96,23 @@ public class ContextSetComparator implements Comparator<ImmutableContextSet> {
throw new AssertionError("sets are equal? " + o1 + " - " + o2); throw new AssertionError("sets are equal? " + o1 + " - " + o2);
} }
@SuppressWarnings("StringEquality") private static final Comparator<Context> CONTEXT_COMPARATOR = ContextSetComparator::compareContexts;
private static final Comparator<String> FAST_STRING_COMPARATOR = (o1, o2) -> o1 == o2 ? 0 : o1.compareTo(o2);
private static final Comparator<Context> CONTEXT_COMPARATOR = (o1, o2) -> { private static int compareContexts(Context o1, Context o2) {
if (o1 == o2) { if (o1 == o2) {
return 0; return 0;
} }
int i = FAST_STRING_COMPARATOR.compare(o1.getKey(), o2.getKey()); int i = compareStringsFast(o1.getKey(), o2.getKey());
if (i != 0) { if (i != 0) {
return i; return i;
} }
return FAST_STRING_COMPARATOR.compare(o1.getValue(), o2.getValue()); return compareStringsFast(o1.getValue(), o2.getValue());
}; }
@SuppressWarnings("StringEquality")
private static int compareStringsFast(String o1, String o2) {
return o1 == o2 ? 0 : o1.compareTo(o2);
}
} }

View File

@ -26,6 +26,7 @@
package me.lucko.luckperms.common.context.contextset; package me.lucko.luckperms.common.context.contextset;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
import com.google.common.collect.SetMultimap; import com.google.common.collect.SetMultimap;
import net.luckperms.api.context.Context; import net.luckperms.api.context.Context;
@ -35,9 +36,12 @@ import net.luckperms.api.context.DefaultContextKeys;
import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.NonNull;
import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Iterator;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.Spliterator;
public abstract class AbstractContextSet implements ContextSet { public abstract class AbstractContextSet implements ContextSet {
@ -91,6 +95,18 @@ public abstract class AbstractContextSet implements ContextSet {
protected abstract boolean otherContainsAll(ContextSet other, ContextSatisfyMode mode); protected abstract boolean otherContainsAll(ContextSet other, ContextSatisfyMode mode);
public abstract Context[] toArray();
@Override
public @NonNull Iterator<Context> iterator() {
return Iterators.forArray(toArray());
}
@Override
public Spliterator<Context> spliterator() {
return Arrays.spliterator(toArray());
}
@Override @Override
public boolean isEmpty() { public boolean isEmpty() {
return backing().isEmpty(); return backing().isEmpty();

View File

@ -25,7 +25,6 @@
package me.lucko.luckperms.common.context.contextset; package me.lucko.luckperms.common.context.contextset;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ImmutableSetMultimap;
@ -42,11 +41,9 @@ import net.luckperms.api.context.MutableContextSet;
import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.NonNull;
import java.util.Collection; import java.util.Collection;
import java.util.Iterator;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.Spliterator;
public final class ImmutableContextSetImpl extends AbstractContextSet implements ImmutableContextSet { public final class ImmutableContextSetImpl extends AbstractContextSet implements ImmutableContextSet {
public static final ImmutableContextSetImpl EMPTY = new ImmutableContextSetImpl(ImmutableSetMultimap.of()); public static final ImmutableContextSetImpl EMPTY = new ImmutableContextSetImpl(ImmutableSetMultimap.of());
@ -64,11 +61,19 @@ public final class ImmutableContextSetImpl extends AbstractContextSet implements
} }
private final ImmutableSetMultimap<String, String> map; private final ImmutableSetMultimap<String, String> map;
private final Context[] array;
private final int hashCode; private final int hashCode;
ImmutableContextSetImpl(ImmutableSetMultimap<String, String> contexts) { ImmutableContextSetImpl(ImmutableSetMultimap<String, String> contexts) {
this.map = contexts; this.map = contexts;
this.hashCode = this.map.hashCode(); this.hashCode = this.map.hashCode();
Set<Map.Entry<String, String>> entries = this.map.entries();
this.array = new Context[entries.size()];
int i = 0;
for (Map.Entry<String, String> e : entries) {
this.array[i++] = new ContextImpl(e.getKey(), e.getValue());
}
} }
@Override @Override
@ -99,12 +104,7 @@ public final class ImmutableContextSetImpl extends AbstractContextSet implements
@Override @Override
public @NonNull Set<Context> toSet() { public @NonNull Set<Context> toSet() {
ImmutableSet.Builder<Context> builder = ImmutableSet.builder(); return ImmutableSet.copyOf(this.array);
Set<Map.Entry<String, String>> entries = this.map.entries();
for (Map.Entry<String, String> e : entries) {
builder.add(new ContextImpl(e.getKey(), e.getValue()));
}
return builder.build();
} }
@Override @Override
@ -122,24 +122,9 @@ public final class ImmutableContextSetImpl extends AbstractContextSet implements
return m.build(); return m.build();
} }
private ImmutableList<Context> toList() {
Set<Map.Entry<String, String>> entries = this.map.entries();
Context[] array = new Context[entries.size()];
int i = 0;
for (Map.Entry<String, String> e : entries) {
array[i++] = new ContextImpl(e.getKey(), e.getValue());
}
return ImmutableList.copyOf(array);
}
@Override @Override
public @NonNull Iterator<Context> iterator() { public Context[] toArray() {
return toList().iterator(); return this.array;
}
@Override
public Spliterator<Context> spliterator() {
return toList().spliterator();
} }
@Override @Override

View File

@ -26,7 +26,6 @@
package me.lucko.luckperms.common.context.contextset; package me.lucko.luckperms.common.context.contextset;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ImmutableSetMultimap;
@ -43,11 +42,9 @@ import net.luckperms.api.context.MutableContextSet;
import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.NonNull;
import java.util.Collection; import java.util.Collection;
import java.util.Iterator;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.Spliterator;
public final class MutableContextSetImpl extends AbstractContextSet implements MutableContextSet { public final class MutableContextSetImpl extends AbstractContextSet implements MutableContextSet {
private final SetMultimap<String, String> map; private final SetMultimap<String, String> map;
@ -132,7 +129,8 @@ public final class MutableContextSetImpl extends AbstractContextSet implements M
return builder.build(); return builder.build();
} }
private ImmutableList<Context> toList() { @Override
public Context[] toArray() {
Set<Map.Entry<String, String>> entries = this.map.entries(); Set<Map.Entry<String, String>> entries = this.map.entries();
Context[] array; Context[] array;
synchronized (this.map) { synchronized (this.map) {
@ -142,17 +140,7 @@ public final class MutableContextSetImpl extends AbstractContextSet implements M
array[i++] = new ContextImpl(e.getKey(), e.getValue()); array[i++] = new ContextImpl(e.getKey(), e.getValue());
} }
} }
return ImmutableList.copyOf(array); return array;
}
@Override
public @NonNull Iterator<Context> iterator() {
return toList().iterator();
}
@Override
public Spliterator<Context> spliterator() {
return toList().spliterator();
} }
@Override @Override

View File

@ -31,7 +31,6 @@ import net.luckperms.api.node.types.PermissionNode;
import java.util.Comparator; import java.util.Comparator;
public class NodeComparator implements Comparator<Node> { public class NodeComparator implements Comparator<Node> {
private static final Comparator<? super Node> INSTANCE = new NodeComparator(); private static final Comparator<? super Node> INSTANCE = new NodeComparator();
private static final Comparator<? super Node> REVERSE = INSTANCE.reversed(); private static final Comparator<? super Node> REVERSE = INSTANCE.reversed();
@ -43,17 +42,20 @@ public class NodeComparator implements Comparator<Node> {
return REVERSE; return REVERSE;
} }
@SuppressWarnings({"ConstantConditions", "OptionalGetWithoutIsPresent"})
@Override @Override
public int compare(Node o1, Node o2) { public int compare(Node o1, Node o2) {
if (o1.equals(o2)) { if (o1.equals(o2)) {
return 0; return 0;
} }
// compare whether nodes are temporary
int result = Boolean.compare(o1.hasExpiry(), o2.hasExpiry()); int result = Boolean.compare(o1.hasExpiry(), o2.hasExpiry());
if (result != 0) { if (result != 0) {
return result; return result;
} }
// compare whether nodes are wildcard nodes
result = Boolean.compare( result = Boolean.compare(
o1 instanceof PermissionNode && ((PermissionNode) o1).isWildcard(), o1 instanceof PermissionNode && ((PermissionNode) o1).isWildcard(),
o2 instanceof PermissionNode && ((PermissionNode) o2).isWildcard() o2 instanceof PermissionNode && ((PermissionNode) o2).isWildcard()
@ -62,6 +64,8 @@ public class NodeComparator implements Comparator<Node> {
return result; return result;
} }
// compare expiry times if both nodes are temporary
// due to the comparison earlier, either both nodes are temporary or neither are.
if (o1.hasExpiry()) { if (o1.hasExpiry()) {
result = o1.getExpiry().compareTo(o2.getExpiry()); result = o1.getExpiry().compareTo(o2.getExpiry());
if (result != 0) { if (result != 0) {
@ -69,20 +73,27 @@ public class NodeComparator implements Comparator<Node> {
} }
} }
if (o1 instanceof PermissionNode && ((PermissionNode) o1).isWildcard()) { // compare wildcard level if both nodes are wildcards
result = Integer.compare(((PermissionNode) o1).getWildcardLevel().getAsInt(), ((PermissionNode) o2).getWildcardLevel().getAsInt()); // due to the comparison earlier, either both nodes are wildcards or neither are
if (o1 instanceof PermissionNode && ((PermissionNode) o1).isWildcard()) { // implies o2.isWildcard too
int o1Level = ((PermissionNode) o1).getWildcardLevel().getAsInt();
int o2Level = ((PermissionNode) o2).getWildcardLevel().getAsInt();
result = Integer.compare(o1Level, o2Level);
if (result != 0) { if (result != 0) {
return result; return result;
} }
} }
// note vvv // compare node keys lexicographically
// note that the order is reversed - A comes before Z
result = -o1.getKey().compareTo(o2.getKey()); result = -o1.getKey().compareTo(o2.getKey());
if (result != 0) { if (result != 0) {
return result; return result;
} }
// note vvv - we want false to have priority // compare the boolean node values
// note that the order is reversed, false comes before true
result = -Boolean.compare(o1.getValue(), o2.getValue()); result = -Boolean.compare(o1.getValue(), o2.getValue());
if (result != 0) { if (result != 0) {
return result; return result;

View File

@ -27,6 +27,7 @@ package me.lucko.luckperms.common.node.comparator;
import me.lucko.luckperms.common.context.ContextSetComparator; import me.lucko.luckperms.common.context.ContextSetComparator;
import net.luckperms.api.context.ImmutableContextSet;
import net.luckperms.api.node.Node; import net.luckperms.api.node.Node;
import java.util.Comparator; import java.util.Comparator;
@ -35,10 +36,8 @@ import java.util.Comparator;
* Compares permission nodes based upon their supposed "priority". * Compares permission nodes based upon their supposed "priority".
*/ */
public class NodeWithContextComparator implements Comparator<Node> { public class NodeWithContextComparator implements Comparator<Node> {
private NodeWithContextComparator() {} private static final Comparator<? super Node> INSTANCE = new NodeWithContextComparator(ContextSetComparator.normal(), NodeComparator.normal());
private static final Comparator<? super Node> REVERSE = new NodeWithContextComparator(ContextSetComparator.reverse(), NodeComparator.reverse());
private static final Comparator<? super Node> INSTANCE = new NodeWithContextComparator();
private static final Comparator<? super Node> REVERSE = INSTANCE.reversed();
public static Comparator<? super Node> normal() { public static Comparator<? super Node> normal() {
return INSTANCE; return INSTANCE;
@ -48,22 +47,26 @@ public class NodeWithContextComparator implements Comparator<Node> {
return REVERSE; return REVERSE;
} }
private final Comparator<? super ImmutableContextSet> contextSetComparator;
private final Comparator<? super Node> nodeComparator;
NodeWithContextComparator(Comparator<? super ImmutableContextSet> contextSetComparator, Comparator<? super Node> nodeComparator) {
this.contextSetComparator = contextSetComparator;
this.nodeComparator = nodeComparator;
}
@Override @Override
public int compare(Node o1, Node o2) { public int compare(Node o1, Node o2) {
if (o1.equals(o2)) { if (o1.equals(o2)) {
return 0; return 0;
} }
int result = ContextSetComparator.normal().compare( int result = this.contextSetComparator.compare(o1.getContexts(), o2.getContexts());
o1.getContexts(),
o2.getContexts()
);
if (result != 0) { if (result != 0) {
return result; return result;
} }
return NodeComparator.normal().compare(o1, o2); return this.nodeComparator.compare(o1, o2);
} }
} }