Skip to content

perf: look up each node's guild and each guild's limits once per pass #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
XxFran10xX wants to merge 1 commit into
base: main
Choose a base branch
from
+116 −23
Draft

perf: look up each node's guild and each guild's limits once per pass #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,10 @@ public static double getTotalUpkeep(Guild g) {
int[] amount = {-1};
java.util.function.IntSupplier count = () -> amount[0] >= 0 ? amount[0] : (amount[0] = getNodeAmount(g));
for (Node n : nodes) {
if (!n.getIsActive() || !n.hasGuild()) continue;
if (!n.getGuild().getId().equalsIgnoreCase(g.getId())) continue;
if (!n.getIsActive()) continue;
// One lookup of the node's guild answers both whether it has one and which it is.
Guild owner = n.getGuild();
if (owner == null || !owner.getId().equalsIgnoreCase(g.getId())) continue;
total += n.getDailyUpkeep(count);
}
return total;
Expand All @@ -174,9 +176,10 @@ public static Integer getNodeAmount(Guild g) {
if(g == null) return i;
if(passCounts != null) return g.getId() == null ? i : passCounts.getOrDefault(g.getId(), i);
for(Node n : nodes) {
if(!n.hasGuild()) continue;
Guild owner = n.getGuild();
if(owner == null) continue;
if(n.getBlock().isSpecial()) continue;
if(n.getGuild().getId().equalsIgnoreCase(g.getId())) i++;
if(owner.getId().equalsIgnoreCase(g.getId())) i++;
}
return i;
}
Expand All @@ -186,28 +189,44 @@ public static Integer getNodeAmount(Guild g) {
* nodes x nodes guild lookups. Counted once per pass instead, and kept right by {@link #released}.
*/
private static Map<String, Integer> passCounts;
/**
* Whether each guild checked so far in one particle pass can hold its nodes. {@link Node#canHold}
* asks only about the guild, so all its nodes get the same answer until one is let go; that
* changes the count and resyncs realms, so {@link #released} forgets the answers.
*/
private static Map<Guild, Boolean> passHolds;

static void beginCountPass() {
Map<String, Integer> counts = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for(Node n : nodes) {
if(!n.hasGuild()) continue;
Guild owner = n.getGuild();
if(owner == null) continue;
if(n.getBlock().isSpecial()) continue;
String id = n.getGuild().getId();
String id = owner.getId();
if(id != null) counts.merge(id, 1, Integer::sum);
}
passCounts = counts;
passHolds = new IdentityHashMap<>();
}

static void endCountPass() {
passCounts = null;
passHolds = null;
}

/** A node of {@code g} was just let go during a particle pass, so its guild holds one fewer. */
public static void released(Guild g, Node n) {
if(passHolds != null) passHolds.clear();
if(passCounts == null || g == null || g.getId() == null || n.getBlock().isSpecial()) return;
passCounts.computeIfPresent(g.getId(), (id, count) -> count - 1);
}

/** {@link Node#canHold} for a node's check: worked out once per guild during a particle pass. */
public static boolean canHold(Node n, Guild g) {
if(passHolds == null) return n.canHold(g);
return passHolds.computeIfAbsent(g, n::canHold);
}

/** Room for new nodes, from the guild's active members. */
public static Integer getNodeCapacity(Guild g) {
return capacityFor(g, net.tfminecraft.dowsing.utils.ActiveMembers.count(g));
Expand Down Expand Up @@ -324,11 +343,15 @@ public void run()
beginCountPass();
try {
for(Node n : nodes){
if(n.isClaimable()){
if(!SimpleFactionsApi.ready()) continue;
// Looked up once here and handed to the check, not once to see if the
// node is claimable and twice more inside the check.
Guild g = n.getGuild();
if(g == null){
Location loc = n.getLoc().clone().add(0.5, 1, 0.5);
loc.getWorld().spawnParticle(Particle.HAPPY_VILLAGER, loc, 10);
} else {
n.check();
n.check(g);
}
}
} finally {
Expand Down
23 changes: 15 additions & 8 deletions src/main/java/net/tfminecraft/dowsing/objects/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,13 @@ Guild resolveGuild() {
public boolean canHold(Guild g){
if(g == null) return false;
if(held(g)) return true;
Player p = Members.player(g.getLeader());
if(p != null && p.isOnline() && Permissions.isAdmin(p)) return true;
// Holding counts every member, active or not; only new claims need active ones.
if(g.getMembers().size() < Cache.minMembersForNode) return false;
if(NodeManager.getNodeAmount(g)-NodeManager.getHeldNodeCapacity(g) > 0) return false;
return true;
if(g.getMembers().size() >= Cache.minMembersForNode
&& NodeManager.getNodeAmount(g)-NodeManager.getHeldNodeCapacity(g) <= 0) return true;
// Short of members or over capacity, only an online admin leader keeps the nodes. Asked last:
// it reads the leader's character, which a guild within its limits never needs.
Player p = Members.player(g.getLeader());
return p != null && p.isOnline() && Permissions.isAdmin(p);
}

/**
Expand Down Expand Up @@ -413,10 +414,16 @@ Integer getNaturalYieldFromChunk(NodeBlock b, Location l) throws NumberFormatExc

public void check() {
if (!SimpleFactionsApi.ready()) return;
if(isClaimable()) return;
Guild g = resolveGuild();
check(resolveGuild());
}

/**
* {@link #check()} for a caller that has just resolved this node's guild, so the particle pass
* looks each node's guild up once instead of three times. Null is an unclaimed node.
*/
public void check(Guild g) {
if(g == null) return;
if(canHold(g)) return;
if(NodeManager.canHold(this, g)) return;
Player p = Members.player(g.getLeader());
if(p != null && p.isOnline()) {
p.sendMessage("§cYou lost control of the "+block.getResource()+" Node §c!");
Expand Down
63 changes: 58 additions & 5 deletions src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void countsCapacityAndUpkeepRespectOwnershipSpecialNodesAndPurchasedLimits() {
assertEquals(1, NodeManager.getNodeCapacity(null)); assertEquals(1, NodeManager.getNodeCapacity(f.guild));
NodeManager.nodes.add(f.node); when(f.node.getIsActive()).thenReturn(true); when(f.node.getDailyUpkeep(any(java.util.function.IntSupplier.class))).thenReturn(2.25);
Node inactive = f.nodeAt(f.location), unowned = f.nodeAt(f.location), other = f.nodeAt(f.location);
when(unowned.hasGuild()).thenReturn(false); when(unowned.getIsActive()).thenReturn(true);
when(unowned.getGuild()).thenReturn(null); when(unowned.getIsActive()).thenReturn(true);
Guild otherGuild = mock(Guild.class); when(otherGuild.getId()).thenReturn("other"); when(other.getGuild()).thenReturn(otherGuild); when(other.getIsActive()).thenReturn(true);
NodeManager.nodes.addAll(List.of(inactive, unowned, other));
assertEquals(2, NodeManager.getNodeAmount(f.guild)); assertEquals(2.25, NodeManager.getTotalUpkeep(f.guild));
Expand Down Expand Up @@ -222,7 +222,7 @@ void cachingReloadsBlockTypeAndPreviouslySelectedProductionMethods() {
@Test
void aParticlePassCountsEachGuildOnceAndTracksNodesLetGo() {
Node second = f.nodeAt(f.location), unowned = f.nodeAt(f.location), other = f.nodeAt(f.location), special = f.nodeAt(f.location), noId = f.nodeAt(f.location);
when(unowned.hasGuild()).thenReturn(false);
when(unowned.getGuild()).thenReturn(null);
Guild otherGuild = mock(Guild.class); when(otherGuild.getId()).thenReturn("OTHER"); when(other.getGuild()).thenReturn(otherGuild);
NodeBlock specialBlock = mock(NodeBlock.class); when(specialBlock.isSpecial()).thenReturn(true); when(special.getBlock()).thenReturn(specialBlock);
Guild idless = mock(Guild.class); when(noId.getGuild()).thenReturn(idless);
Expand All @@ -233,7 +233,7 @@ void aParticlePassCountsEachGuildOnceAndTracksNodesLetGo() {
try {
assertEquals(2, NodeManager.getNodeAmount(f.guild)); assertEquals(1, NodeManager.getNodeAmount(otherGuild));
assertEquals(0, NodeManager.getNodeAmount(idless)); assertEquals(0, NodeManager.getNodeAmount(null));
when(f.node.hasGuild()).thenReturn(false);
when(f.node.getGuild()).thenReturn(null);
assertEquals(2, NodeManager.getNodeAmount(f.guild), "counted once at the start of the pass");
NodeManager.released(f.guild, f.node); assertEquals(1, NodeManager.getNodeAmount(f.guild));
NodeManager.released(f.guild, special); NodeManager.released(null, f.node); NodeManager.released(idless, f.node);
Expand Down Expand Up @@ -266,15 +266,68 @@ void aRealCheckThatLetsANodeGoLowersItsGuildsCountForTheRestOfThePass() {
}
}

@Test
void aGuildsRightToItsNodesIsWorkedOutOncePerPassAndAgainAfterOneIsLetGo() {
Cache.extraCapacity = false; NodeManager.extraCapacityByGuild.put("guild", 1);
Node first = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25);
Node second = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25);
Node third = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25);
NodeManager.nodes.addAll(List.of(first, second));
clearInvocations(f.guild);
first.check(); second.check();
verify(f.guild, times(2)).isHeld();
NodeManager.beginCountPass();
try {
first.check(); second.check();
verify(f.guild, times(3)).isHeld();
} finally {
NodeManager.endCountPass();
}
NodeManager.nodes.add(third);
NodeManager.beginCountPass();
try {
first.check(); second.check(); third.check();
// Over capacity: the first node goes, then the guild is judged afresh, once for the other two.
verify(f.guild, times(5)).isHeld();
assertTrue(first.isClaimable()); assertFalse(second.isClaimable()); assertFalse(third.isClaimable());
} finally {
NodeManager.endCountPass();
}
second.check();
verify(f.guild, times(6)).isHeld();
}

@Test
void aParticlePassLooksUpEachNodesGuildOnceToCountAndOnceToCheck() {
Cache.extraCapacity = false; NodeManager.extraCapacityByGuild.put("guild", 1);
Node first = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25);
Node second = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25);
Node unclaimed = new Node(UUID.randomUUID(), f.block, f.location, null, false, 1, 0, f.type, 60, 0, 25);
NodeManager.nodes.addAll(List.of(first, second, unclaimed));
try (Cycles cycles = new Cycles()) {
f.manager.start();
f.factions.clearInvocations(); clearInvocations(f.guild);
cycles.tasks.get(5L).run();
f.factions.verify(() -> FactionManager.getGuildByString("guild"), times(4));
verify(f.guild, times(1)).isHeld();
verify(f.world, times(1)).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10);
assertFalse(first.isClaimable()); assertFalse(second.isClaimable());
}
}

@Test
void timersUseTheirIntendedCadenceAndRunParticlesChecksAndEfficiencyGrowth() {
Node claimed = f.node, unclaimed = f.nodeAt(f.location);
when(unclaimed.isClaimable()).thenReturn(true); NodeManager.nodes.addAll(List.of(claimed, unclaimed));
when(unclaimed.getGuild()).thenReturn(null); NodeManager.nodes.addAll(List.of(claimed, unclaimed));
try (Cycles cycles = new Cycles()) {
f.manager.start(); assertEquals(java.util.Set.of(5L, 600L, 1200L, 72000L), cycles.tasks.keySet());
cycles.tasks.get(5L).run(); verify(claimed).check();
cycles.tasks.get(5L).run(); verify(claimed).check(f.guild); verify(unclaimed, never()).check(any());
verify(f.world).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10);
cycles.tasks.get(72000L).run(); verify(claimed).growEfficiency(); verify(unclaimed).growEfficiency();
// While SimpleFactions is not ready a pass neither checks nodes nor marks them claimable.
net.tfminecraft.dowsing.testsupport.FactionFixture.api.when(net.tfminecraft.simplefactions.api.SimpleFactionsApi::ready).thenReturn(false);
cycles.tasks.get(5L).run(); verify(claimed, times(1)).check(f.guild);
verify(f.world, times(1)).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10);
}
}

Expand Down
14 changes: 12 additions & 2 deletions src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,20 @@ void guildNoticesReachOnlyOnlineMembersOfTheOwningGuild() {
}

@Test
void deletedOwnerDuringResolutionLeavesTheNodeClaimable() {
void aCheckLooksItsOwnerUpOnceAndADeletedOwnerLeavesTheNodeClaimable() {
Node node = f.owned();
f.factions.when(() -> FactionManager.getGuildByString("guild")).thenReturn(f.guild).thenReturn(null);
node.check(); assertTrue(node.isClaimable());
node.check(); assertSame(f.guild, node.guild, "one lookup per check");
node.check(); assertNull(node.guild); assertTrue(node.isClaimable());
}

@Test
void aGuildWithinItsLimitsKeepsItsNodesWithoutItsLeaderBeingLookedUp() {
Node node = f.owned();
clearInvocations(f.guild);
assertTrue(node.canHold(f.guild)); verify(f.guild, never()).getLeader();
Cache.minMembersForNode = 3;
assertFalse(node.canHold(f.guild)); verify(f.guild).getLeader();
}

@Test
Expand Down