From 9e3927ed6324fabc822e7885e45da054fc655609 Mon Sep 17 00:00:00 2001 From: William Date: Thu, 4 Apr 2024 18:04:17 +0100 Subject: [PATCH] refactor: refactor claim caching, improve perf O(n) to O(1). Thanks @Al3xDev for the idea. --- .../husktowns/api/HuskTownsAPI.java | 7 +- .../net/william278/husktowns/claim/Chunk.java | 27 ++-- .../husktowns/claim/ClaimWorld.java | 142 +++++++++++------- .../husktowns/command/TownCommand.java | 3 +- .../william278/husktowns/hook/MapHook.java | 19 +-- .../husktowns/migrator/LegacyMigrator.java | 8 +- .../william278/husktowns/util/DataPruner.java | 9 +- 7 files changed, 116 insertions(+), 99 deletions(-) diff --git a/common/src/main/java/net/william278/husktowns/api/HuskTownsAPI.java b/common/src/main/java/net/william278/husktowns/api/HuskTownsAPI.java index f74b87fb..f04068ae 100644 --- a/common/src/main/java/net/william278/husktowns/api/HuskTownsAPI.java +++ b/common/src/main/java/net/william278/husktowns/api/HuskTownsAPI.java @@ -45,7 +45,6 @@ import java.util.*; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Consumer; /** @@ -415,11 +414,7 @@ public void updateClaim(@NotNull TownClaim claim, @NotNull World world) throws I if (claim.isAdminClaim(plugin)) { return; } - - final ConcurrentLinkedQueue claims = claimWorld.getClaims() - .computeIfAbsent(claim.town().getId(), k -> new ConcurrentLinkedQueue<>()); - claims.removeIf(c -> c.getChunk().equals(claim.claim().getChunk())); - claims.add(claim.claim()); + claimWorld.replaceClaim(claim, plugin); plugin.getDatabase().updateClaimWorld(claimWorld); }); } diff --git a/common/src/main/java/net/william278/husktowns/claim/Chunk.java b/common/src/main/java/net/william278/husktowns/claim/Chunk.java index 623fbbec..ad81ebc2 100644 --- a/common/src/main/java/net/william278/husktowns/claim/Chunk.java +++ b/common/src/main/java/net/william278/husktowns/claim/Chunk.java @@ -20,10 +20,14 @@ package net.william278.husktowns.claim; import com.google.gson.annotations.Expose; +import lombok.Getter; +import lombok.NoArgsConstructor; import net.william278.cloplib.operation.OperationChunk; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +@Getter +@NoArgsConstructor public class Chunk implements OperationChunk { @Expose @@ -41,18 +45,6 @@ public static Chunk at(int x, int z) { return new Chunk(x, z); } - @SuppressWarnings("unused") - private Chunk() { - } - - public int getX() { - return x; - } - - public int getZ() { - return z; - } - @Override public boolean equals(@Nullable Object obj) { if (this == obj) return true; @@ -66,12 +58,21 @@ public String toString() { return "(x: " + x + ", z: " + z + ")"; } + /** + * Get the long position of the chunk + * + * @return the long position + */ + public long asLong() { + return ((long) x << 32) | (z & 0xffffffffL); + } + public int distanceBetween(@NotNull Chunk chunk) { return Math.abs(x - chunk.x) + Math.abs(z - chunk.z); } public boolean contains(Position position) { return position.getX() >= x * 16 && position.getX() < (x + 1) * 16 - && position.getZ() >= z * 16 && position.getZ() < (z + 1) * 16; + && position.getZ() >= z * 16 && position.getZ() < (z + 1) * 16; } } diff --git a/common/src/main/java/net/william278/husktowns/claim/ClaimWorld.java b/common/src/main/java/net/william278/husktowns/claim/ClaimWorld.java index 6e8f3fd6..3d10b95a 100644 --- a/common/src/main/java/net/william278/husktowns/claim/ClaimWorld.java +++ b/common/src/main/java/net/william278/husktowns/claim/ClaimWorld.java @@ -19,34 +19,40 @@ package net.william278.husktowns.claim; +import com.google.common.collect.Maps; +import com.google.common.collect.Queues; import com.google.gson.annotations.Expose; import com.google.gson.annotations.SerializedName; +import lombok.Getter; +import lombok.NoArgsConstructor; import net.william278.husktowns.HuskTowns; import net.william278.husktowns.town.Town; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Unmodifiable; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; +import java.util.*; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentMap; +import java.util.stream.Collectors; +@NoArgsConstructor public class ClaimWorld { + @Getter private int id; @Expose - private ConcurrentHashMap> claims; - + @SerializedName("claims") + private ConcurrentMap> claims = Maps.newConcurrentMap(); @Expose @SerializedName("admin_claims") - private ConcurrentLinkedQueue adminClaims; + private ConcurrentLinkedQueue adminClaims = Queues.newConcurrentLinkedQueue(); + + @Expose(deserialize = false, serialize = false) + private transient Map cachedClaims = Maps.newConcurrentMap(); private ClaimWorld(int id, @NotNull Map> claims, @NotNull List adminClaims) { this.id = id; - this.adminClaims = new ConcurrentLinkedQueue<>(adminClaims); - this.claims = new ConcurrentHashMap<>(); - claims.forEach((key, value) -> this.claims.put(key, new ConcurrentLinkedQueue<>(value))); + this.cacheClaims(claims, adminClaims); } @NotNull @@ -54,32 +60,19 @@ public static ClaimWorld of(int id, @NotNull Map> claims, @ return new ClaimWorld(id, claims, adminClaims); } - @SuppressWarnings("unused") - private ClaimWorld() { + private void cacheClaims(@NotNull Map> claims, @NotNull List adminClaims) { + claims.forEach((key, value) -> { + this.claims.put(key, new ConcurrentLinkedQueue<>(value)); + value.forEach(claim -> this.cachedClaims.put(claim.getChunk().asLong(), new CachedClaim(key, claim))); + }); + adminClaims.forEach(claim -> { + this.cachedClaims.put(claim.getChunk().asLong(), new CachedClaim(-1, claim)); + this.adminClaims.add(claim); + }); } public Optional getClaimAt(@NotNull Chunk chunk, @NotNull HuskTowns plugin) { - return claims.entrySet().stream() - .filter(entry -> entry.getValue().stream().anyMatch(claim -> claim.getChunk().equals(chunk))) - .findFirst() - .flatMap(entry -> entry.getValue().stream() - .filter(claim -> claim.getChunk().equals(chunk)) - .findFirst() - .flatMap(claim -> plugin.findTown(entry.getKey()) - .map(town1 -> new TownClaim(town1, claim)))) - .or(() -> adminClaims.stream() - .filter(claim -> claim.getChunk().equals(chunk)) - .findFirst() - .map(claim -> new TownClaim(plugin.getAdminTown(), claim))); - } - - /** - * Get the ID of the claim world - * - * @return the ID of the claim world - */ - public int getId() { - return id; + return Optional.ofNullable(cachedClaims.get(chunk.asLong())).map(cached -> cached.getTownClaim(plugin)); } /** @@ -97,7 +90,7 @@ public void updateId(int id) { * @return the number of claims in this world */ public int getClaimCount() { - return claims.values().stream().mapToInt(ConcurrentLinkedQueue::size).sum() + getAdminClaimCount(); + return cachedClaims.size(); } /** @@ -109,6 +102,29 @@ public int getAdminClaimCount() { return adminClaims.size(); } + @NotNull + public List getTownClaims(int townId, @NotNull HuskTowns plugin) { + return cachedClaims.values().stream() + .filter(cachedClaim -> cachedClaim.townId == townId) + .map(cachedClaim -> cachedClaim.getTownClaim(plugin)) + .collect(Collectors.toList()); + } + + @NotNull + @Unmodifiable + public Map> getClaims() { + return claims.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> new ArrayList<>(entry.getValue()))); + } + + @NotNull + public List getClaims(@NotNull HuskTowns plugin) { + return cachedClaims.values().stream() + .map(cachedClaim -> cachedClaim.getTownClaim(plugin)) + .collect(Collectors.toList()); + } + + /** * Remove claims by a town on this world * @@ -119,6 +135,7 @@ public int removeTownClaims(int townId) { if (claims.containsKey(townId)) { int claimCount = claims.get(townId).size(); claims.remove(townId); + cachedClaims.values().removeIf(cachedClaim -> cachedClaim.townId == townId); return claimCount; } return 0; @@ -129,18 +146,39 @@ public void addClaim(@NotNull TownClaim townClaim) { claims.put(townClaim.town().getId(), new ConcurrentLinkedQueue<>()); } claims.get(townClaim.town().getId()).add(townClaim.claim()); + cachedClaims.put(townClaim.claim().getChunk().asLong(), new CachedClaim(townClaim.town().getId(), townClaim.claim())); + } + + public void replaceClaim(@NotNull TownClaim townClaim, @NotNull HuskTowns plugin) { + final Claim claim = townClaim.claim(); + if (townClaim.isAdminClaim(plugin)) { + adminClaims.removeIf(c -> c.getChunk().equals(claim.getChunk())); + adminClaims.add(claim); + cachedClaims.put(claim.getChunk().asLong(), new CachedClaim(-1, claim)); + } else if (claims.containsKey(townClaim.town().getId())) { + claims.get(townClaim.town().getId()).removeIf(c -> c.getChunk().equals(claim.getChunk())); + claims.get(townClaim.town().getId()).add(claim); + cachedClaims.put(claim.getChunk().asLong(), new CachedClaim(townClaim.town().getId(), claim)); + } } public void addAdminClaim(@NotNull Claim claim) { + cachedClaims.put(claim.getChunk().asLong(), new CachedClaim(-1, claim)); adminClaims.add(claim); } public void removeClaim(@NotNull Town town, @NotNull Chunk chunk) { + cachedClaims.remove(chunk.asLong()); if (claims.containsKey(town.getId())) { claims.get(town.getId()).removeIf(claim -> claim.getChunk().equals(chunk)); } } + public void removeAdminClaim(@NotNull Chunk chunk) { + cachedClaims.remove(chunk.asLong()); + adminClaims.removeIf(claim -> claim.getChunk().equals(chunk)); + } + @NotNull public List getClaimsNear(@NotNull Chunk chunk, int radius, @NotNull HuskTowns plugin) { if (radius <= 0) { @@ -161,24 +199,12 @@ public List getAdjacentClaims(@NotNull Chunk chunk, @NotNull HuskTown return getClaimsNear(chunk, 1, plugin); } - public void removeAdminClaim(@NotNull Chunk chunk) { - adminClaims.removeIf(claim -> claim.getChunk().equals(chunk)); - } - - @NotNull - public ConcurrentHashMap> getClaims() { - return claims; - } - @NotNull - public List getClaims(@NotNull HuskTowns plugin) { - List townClaims = new ArrayList<>(); - claims.forEach((townId, claimList) -> { - Optional town = plugin.findTown(townId); - town.ifPresent(value -> claimList.forEach(claim -> townClaims.add(new TownClaim(value, claim)))); - }); - adminClaims.forEach(claim -> townClaims.add(new TownClaim(plugin.getAdminTown(), claim))); - return townClaims; + public boolean pruneOrphanClaims(@NotNull HuskTowns plugin) { + return new HashMap<>(claims).keySet().stream() + .filter(town -> plugin.findTown(town).isEmpty()) + .map(this::removeTownClaims) + .anyMatch(count -> count > 0); } @Override @@ -189,4 +215,16 @@ public boolean equals(Object obj) { return id == claimWorld.id; } + private record CachedClaim(int townId, @NotNull Claim claim) { + @NotNull + TownClaim getTownClaim(@NotNull HuskTowns plugin) { + if (townId == -1) { + return new TownClaim(plugin.getAdminTown(), claim); + } + return plugin.findTown(townId) + .map(town -> new TownClaim(town, claim)) + .orElseThrow(() -> new IllegalStateException("Claim has invalid town ID: " + townId)); + } + } + } diff --git a/common/src/main/java/net/william278/husktowns/command/TownCommand.java b/common/src/main/java/net/william278/husktowns/command/TownCommand.java index 5790fabb..1dbee56c 100644 --- a/common/src/main/java/net/william278/husktowns/command/TownCommand.java +++ b/common/src/main/java/net/william278/husktowns/command/TownCommand.java @@ -46,7 +46,6 @@ import java.math.BigDecimal; import java.time.format.DateTimeFormatter; import java.util.*; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -191,7 +190,7 @@ public void execute(@NotNull CommandUser executor, @NotNull String[] args) { final Component mapGrid = plugin.getWorlds().stream() .map(world -> Map.entry(world, plugin.getClaimWorld(world) .map(claimWorld -> claimWorld.getClaims().get(town.getId())) - .orElse(new ConcurrentLinkedQueue<>()).stream() + .orElse(new ArrayList<>()).stream() .map(claim -> new TownClaim(town, claim)) .toList())) .flatMap((worldMap) -> worldMap.getValue().stream() diff --git a/common/src/main/java/net/william278/husktowns/hook/MapHook.java b/common/src/main/java/net/william278/husktowns/hook/MapHook.java index 5560a2d3..66df453f 100644 --- a/common/src/main/java/net/william278/husktowns/hook/MapHook.java +++ b/common/src/main/java/net/william278/husktowns/hook/MapHook.java @@ -26,7 +26,6 @@ import org.jetbrains.annotations.NotNull; import java.util.List; -import java.util.concurrent.ConcurrentLinkedQueue; public abstract class MapHook extends Hook { @@ -39,23 +38,17 @@ protected MapHook(@NotNull HuskTowns plugin, @NotNull String name) { public abstract void removeClaimMarker(@NotNull TownClaim claim, @NotNull World world); public final void removeClaimMarkers(@NotNull Town town) { - plugin.getWorlds().forEach(world -> plugin.getClaimWorld(world).ifPresent(claimWorld -> { - final List claims = claimWorld.getClaims() - .getOrDefault(town.getId(), new ConcurrentLinkedQueue<>()).stream() - .map(claim -> new TownClaim(town, claim)).toList(); - removeClaimMarkers(claims, world); - })); + plugin.getWorlds().forEach(world -> plugin.getClaimWorld(world).ifPresent( + claimWorld -> removeClaimMarkers(claimWorld.getTownClaims(town.getId(), plugin), world) + )); } public abstract void setClaimMarkers(@NotNull List claims, @NotNull World world); public final void setClaimMarkers(@NotNull Town town) { - plugin.getWorlds().forEach(world -> plugin.getClaimWorld(world).ifPresent(claimWorld -> { - final List claims = claimWorld.getClaims() - .getOrDefault(town.getId(), new ConcurrentLinkedQueue<>()).stream() - .map(claim -> new TownClaim(town, claim)).toList(); - setClaimMarkers(claims, world); - })); + plugin.getWorlds().forEach(world -> plugin.getClaimWorld(world).ifPresent( + claimWorld -> setClaimMarkers(claimWorld.getTownClaims(town.getId(), plugin), world) + )); } public abstract void removeClaimMarkers(@NotNull List claims, @NotNull World world); diff --git a/common/src/main/java/net/william278/husktowns/migrator/LegacyMigrator.java b/common/src/main/java/net/william278/husktowns/migrator/LegacyMigrator.java index 3285e27d..5164813b 100644 --- a/common/src/main/java/net/william278/husktowns/migrator/LegacyMigrator.java +++ b/common/src/main/java/net/william278/husktowns/migrator/LegacyMigrator.java @@ -34,7 +34,6 @@ import java.sql.*; import java.time.ZoneOffset; import java.util.*; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.logging.Level; /** @@ -250,12 +249,7 @@ protected Map getConvertedClaimWorlds() { continue; } - final int townId = town.get().getId(); - if (claimWorld.getClaims().containsKey(townId)) { - claimWorld.getClaims().get(townId).add(claim); - } else { - claimWorld.getClaims().put(townId, new ConcurrentLinkedQueue<>(List.of(claim))); - } + claimWorld.addClaim(new TownClaim(town.get(), claim)); claimWorlds.replaceAll((k, v) -> k.equals(serverWorld) ? claimWorld : v); } } diff --git a/common/src/main/java/net/william278/husktowns/util/DataPruner.java b/common/src/main/java/net/william278/husktowns/util/DataPruner.java index 9e211bdb..7d32a35f 100644 --- a/common/src/main/java/net/william278/husktowns/util/DataPruner.java +++ b/common/src/main/java/net/william278/husktowns/util/DataPruner.java @@ -51,12 +51,9 @@ default void pruneOrphanClaims() { getPlugin().log(Level.INFO, "Validating and pruning orphan claims..."); final LocalTime startTime = LocalTime.now(); - getPlugin().getClaimWorlds().values().forEach(world -> { - world.getClaims().keySet().removeIf( - claim -> getPlugin().getTowns().stream().noneMatch(town -> town.getId() == claim) - ); - getPlugin().getDatabase().updateClaimWorld(world); - }); + getPlugin().getClaimWorlds().values().stream() + .filter(world -> world.pruneOrphanClaims(getPlugin())) + .forEach(w -> getPlugin().getDatabase().updateClaimWorld(w)); getPlugin().log(Level.INFO, "Successfully validated and pruned orphan claims in " + (ChronoUnit.MILLIS.between(startTime, LocalTime.now()) / 1000d) + " seconds");