From 2d807598b58bf5ada97879d5c704c9bdc3893ef4 Mon Sep 17 00:00:00 2001 From: XxFran10xX Date: Wed, 11 Nov 2026 18:42:13 +0100 Subject: [PATCH] perf: look up each node's guild and each guild's limits once per pass The particle pass runs four times a second and asked SimpleFactions for the same things several times per node. A spark profile of Main (60 players, one hour) put it at 6.5 s of the server thread, with the upkeep total SimpleFactions reads at another 2.7 s. - The pass looks a node's guild up once and hands it to the check, instead of once to see if the node is claimable and twice more inside the check. The count at the start of the pass looks it up once instead of twice. - Whether a guild can hold its nodes depends only on the guild, so a pass works it out once per guild instead of once per node. Letting a node go changes the count and resyncs realms, so the answers are forgotten then. - A guild within its member and capacity limits keeps its nodes without its leader being looked up. The lookup reads the leader's character and only matters for a guild that would otherwise lose the node. - The upkeep total and the node count look each node's guild up once instead of twice. Nothing is kept between passes, so a deleted or renamed guild and a node that changes owner are seen exactly as before. Co-Authored-By: Claude Fable 5.2 --- .../dowsing/managers/NodeManager.java | 39 +++++++++--- .../net/tfminecraft/dowsing/objects/Node.java | 23 ++++--- .../managers/NodeManagerCyclesTest.java | 63 +++++++++++++++++-- .../dowsing/objects/NodeBehaviorTest.java | 14 ++++- 4 files changed, 116 insertions(+), 23 deletions(-) diff --git a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java index 78717b9..0ffa8cd 100644 --- a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java +++ b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java @@ -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; @@ -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; } @@ -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 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 passHolds; static void beginCountPass() { Map 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)); @@ -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 { diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index c16bdbd..fd2ceea 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -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); } /** @@ -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!"); diff --git a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java index 79f0dc8..88ed2a1 100644 --- a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java +++ b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java @@ -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)); @@ -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); @@ -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); @@ -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); } } diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java index d9e6511..26cd07e 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java @@ -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