Skip to content

Commit e1ebc6e

Browse files
pupnewfsterpupnewfster
committed
Improve memory churn of path caching by making cached paths immutable and using the cached list directly in the destination logic rather than copying it
1 parent 4df3d6b commit e1ebc6e

File tree

2 files changed

+72
-79
lines changed

2 files changed

+72
-79
lines changed

‎src/main/java/mekanism/common/content/transporter/PathfinderCache.java

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
package mekanism.common.content.transporter;
22

33
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
4+
import java.util.HashMap;
45
import java.util.List;
56
import java.util.Map;
67
import java.util.Set;
78
import java.util.UUID;
89
import mekanism.common.content.network.InventoryNetwork;
910
import mekanism.common.content.network.transmitter.LogisticalTransporterBase;
11+
import mekanism.common.content.transporter.TransporterPathfinder.Pathfinder;
1012
import net.minecraft.core.BlockPos;
1113
import net.minecraft.core.Direction;
14+
import org.jetbrains.annotations.Nullable;
1215

1316
public class PathfinderCache {
1417

@@ -23,19 +26,26 @@ public static void onChanged(InventoryNetwork... networks) {
2326
}
2427
}
2528

26-
public static void addCachedPath(LogisticalTransporterBase start, PathData data, List<BlockPos> positions, double cost) {
27-
cachedPaths.computeIfAbsent(start.getTransmitterNetwork().getUUID(), uuid -> new Object2ObjectOpenHashMap<>()).put(data, new CachedPath(positions, cost));
29+
public static CachedPath addCachedPath(LogisticalTransporterBase start, BlockPos destination, Pathfinder pathfinder) {
30+
CachedPath cachedPath = new CachedPath(pathfinder.getPath(), pathfinder.getFinalScore());
31+
PathData data = new PathData(start.getBlockPos(), destination, pathfinder.getSide());
32+
cachedPaths.computeIfAbsent(start.getTransmitterNetwork().getUUID(), uuid -> new HashMap<>()).put(data, cachedPath);
33+
return cachedPath;
2834
}
2935

36+
@Nullable
3037
public static CachedPath getCache(LogisticalTransporterBase start, BlockPos end, Set<Direction> sides) {
3138
CachedPath ret = null;
3239
UUID uuid = start.getTransmitterNetwork().getUUID();
33-
if (cachedPaths.containsKey(uuid)) {
34-
Map<PathData, CachedPath> pathMap = cachedPaths.get(uuid);
40+
Map<PathData, CachedPath> pathMap = cachedPaths.get(uuid);
41+
if (pathMap != null) {
42+
BlockPos startPos = start.getBlockPos();
3543
for (Direction side : sides) {
36-
CachedPath test = pathMap.get(new PathData(start.getBlockPos(), end, side));
37-
if (ret == null || (test != null && test.cost() < ret.cost())) {
38-
ret = test;
44+
CachedPath test = pathMap.get(new PathData(startPos, end, side));
45+
if (test != null) {
46+
if (ret == null || test.cost() < ret.cost()) {
47+
ret = test;
48+
}
3949
}
4050
}
4151
}
@@ -49,32 +59,6 @@ public static void reset() {
4959
public record CachedPath(List<BlockPos> path, double cost) {
5060
}
5161

52-
public static class PathData {
53-
54-
private final BlockPos startTransporter;
55-
private final BlockPos end;
56-
private final Direction endSide;
57-
private final int hash;
58-
59-
public PathData(BlockPos s, BlockPos e, Direction es) {
60-
startTransporter = s;
61-
end = e;
62-
endSide = es;
63-
int code = 1;
64-
code = 31 * code + startTransporter.hashCode();
65-
code = 31 * code + end.hashCode();
66-
code = 31 * code + endSide.hashCode();
67-
hash = code;
68-
}
69-
70-
@Override
71-
public boolean equals(Object obj) {
72-
return obj instanceof PathData data && data.startTransporter.equals(startTransporter) && data.end.equals(end) && data.endSide == endSide;
73-
}
74-
75-
@Override
76-
public int hashCode() {
77-
return hash;
78-
}
62+
private record PathData(BlockPos startTransporter, BlockPos end, Direction endSide) {
7963
}
8064
}

‎src/main/java/mekanism/common/content/transporter/TransporterPathfinder.java

Lines changed: 54 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package mekanism.common.content.transporter;
22

3+
import com.google.common.collect.ImmutableList;
34
import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
45
import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
56
import it.unimi.dsi.fastutil.objects.Object2DoubleOpenHashMap;
@@ -16,8 +17,6 @@
1617
import mekanism.common.content.network.InventoryNetwork.AcceptorData;
1718
import mekanism.common.content.network.transmitter.LogisticalTransporterBase;
1819
import mekanism.common.content.transporter.PathfinderCache.CachedPath;
19-
import mekanism.common.content.transporter.PathfinderCache.PathData;
20-
import mekanism.common.content.transporter.TransporterPathfinder.Pathfinder.DestChecker;
2120
import mekanism.common.content.transporter.TransporterStack.Path;
2221
import mekanism.common.lib.SidedBlockPos;
2322
import mekanism.common.lib.inventory.IAdvancedTransportEjector;
@@ -30,6 +29,7 @@
3029
import net.minecraft.core.BlockPos;
3130
import net.minecraft.core.Direction;
3231
import net.minecraft.core.GlobalPos;
32+
import net.minecraft.world.item.ItemStack;
3333
import net.minecraft.world.level.Level;
3434
import net.minecraft.world.level.block.entity.BlockEntity;
3535
import net.minecraft.world.level.chunk.ChunkAccess;
@@ -82,19 +82,13 @@ private static Destination getPath(InventoryNetwork network, AcceptorData data,
8282
BlockPos dest = data.getLocation();
8383
CachedPath test = PathfinderCache.getCache(start, dest, data.getSides());
8484
if (test != null && checkPath(network, test.path(), stack)) {
85-
return new Destination(test.path(), false, response, test.cost());
85+
return new Destination(test, response);
8686
}
87-
Pathfinder p = new Pathfinder(new DestChecker() {
88-
@Override
89-
public boolean isValid(Level level, BlockPos pos, @Nullable BlockEntity tile, TransporterStack stack, Direction side) {
90-
return TransporterUtils.canInsert(level, pos, tile, stack.color, response.getStack(), side, false);
91-
}
92-
}, network, start.getLevel(), dest, start.getBlockPos(), stack);
87+
Pathfinder p = new Pathfinder(network, start.getLevel(), dest, start.getBlockPos(), stack, response.getStack(),
88+
(level, pos, tile, s, resp, side) -> TransporterUtils.canInsert(level, pos, tile, s.color, resp, side, false));
9389
p.find(chunkMap);
94-
List<BlockPos> path = p.getPath();
95-
if (path.size() >= 2) {
96-
PathfinderCache.addCachedPath(start, new PathData(start.getBlockPos(), dest, p.getSide()), path, p.finalScore);
97-
return new Destination(path, false, response, p.finalScore);
90+
if (p.hasPath()) {
91+
return new Destination(PathfinderCache.addCachedPath(start, dest, p), response);
9892
}
9993
}
10094
return null;
@@ -177,16 +171,12 @@ public static IdlePathData getIdlePath(LogisticalTransporterBase start, Transpor
177171
}
178172
if (stack.homeLocation != null) {
179173
Long2ObjectMap<ChunkAccess> chunkMap = new Long2ObjectOpenHashMap<>();
180-
Pathfinder p = new Pathfinder(new DestChecker() {
181-
@Override
182-
public boolean isValid(Level level, BlockPos pos, @Nullable BlockEntity tile, TransporterStack stack, Direction side) {
183-
return TransporterUtils.canInsert(level, pos, tile, stack.color, stack.itemStack, side, true);
184-
}
185-
}, network, start.getLevel(), stack.homeLocation, start.getBlockPos(), stack);
174+
//We are idling use the base stack
175+
Pathfinder p = new Pathfinder(network, start.getLevel(), stack.homeLocation, start.getBlockPos(), stack, stack.itemStack,
176+
(level, pos, tile, s, resp, side) -> TransporterUtils.canInsert(level, pos, tile, s.color, resp, side, true));
186177
p.find(chunkMap);
187-
List<BlockPos> path = p.getPath();
188-
if (path.size() >= 2) {
189-
return new IdlePathData(path, Path.HOME);
178+
if (p.hasPath()) {
179+
return new IdlePathData(p.getPath(), Path.HOME);
190180
}
191181
stack.homeLocation = null;
192182
}
@@ -212,7 +202,7 @@ public IdlePath(InventoryNetwork network, BlockPos start, TransporterStack stack
212202
}
213203

214204
public Destination find() {
215-
ArrayList<BlockPos> ret = new ArrayList<>();
205+
List<BlockPos> ret = new ArrayList<>();
216206
ret.add(start);
217207
LogisticalTransporterBase startTransmitter = network.getTransmitter(start);
218208
if (transportStack.idleDir == null) {
@@ -221,7 +211,7 @@ public Destination find() {
221211
LogisticalTransporterBase transmitter = network.getTransmitter(start.relative(transportStack.idleDir));
222212
if (transportStack.canInsertToTransporter(transmitter, transportStack.idleDir, startTransmitter)) {
223213
loopSide(ret, transportStack.idleDir, startTransmitter);
224-
return new Destination(ret, true, null, 0).setPathType(Path.NONE);
214+
return createDestination(ret);
225215
}
226216
TransitRequest request = TransitRequest.simple(transportStack.itemStack);
227217
if (startTransmitter != null) {
@@ -251,15 +241,21 @@ private Destination getDestination(List<BlockPos> ret, @Nullable LogisticalTrans
251241
// is gracefully handled
252242
transportStack.idleDir = side;
253243
ret.add(start.relative(side));
254-
return new Destination(ret, true, null, 0).setPathType(Path.NONE);
244+
return createDestination(ret);
255245
}
256246
}
257247
}
258248
return null;
259249
}
260250
transportStack.idleDir = newSide;
261251
loopSide(ret, newSide, startTransmitter);
262-
return new Destination(ret, true, null, 0).setPathType(Path.NONE);
252+
return createDestination(ret);
253+
}
254+
255+
private Destination createDestination(List<BlockPos> ret) {
256+
List<BlockPos> path = new ArrayList<>(ret);
257+
Collections.reverse(path);
258+
return new Destination(Collections.unmodifiableList(path), null, 0);
263259
}
264260

265261
private void loopSide(List<BlockPos> list, Direction side, @Nullable LogisticalTransporterBase startTransmitter) {
@@ -304,16 +300,22 @@ public static class Destination implements Comparable<Destination> {
304300

305301
private final TransitResponse response;
306302
private final List<BlockPos> path;
303+
private final int cachedHash;
307304
private final double score;
308-
private Path pathType;
305+
private Path pathType = Path.NONE;
309306

310-
public Destination(List<BlockPos> list, boolean inv, TransitResponse ret, double gScore) {
311-
path = new ArrayList<>(list);
312-
if (inv) {
313-
Collections.reverse(path);
314-
}
315-
response = ret;
316-
score = gScore;
307+
public Destination(CachedPath path, TransitResponse ret) {
308+
this(path.path(), ret, path.cost());
309+
}
310+
311+
/**
312+
* @apiNote Expects list to be unmodifiable/immutable (at the very least not mutated after being passed).
313+
*/
314+
public Destination(List<BlockPos> path, TransitResponse ret, double gScore) {
315+
this.path = path;
316+
this.cachedHash = this.path.hashCode();
317+
this.response = ret;
318+
this.score = gScore;
317319
}
318320

319321
public Destination setPathType(Path type) {
@@ -323,9 +325,7 @@ public Destination setPathType(Path type) {
323325

324326
@Override
325327
public int hashCode() {
326-
int code = 1;
327-
code = 31 * code + path.hashCode();
328-
return code;
328+
return cachedHash;
329329
}
330330

331331
@Override
@@ -367,19 +367,21 @@ public static class Pathfinder {
367367
private final BlockPos start;
368368
private final BlockPos finalNode;
369369
private final TransporterStack transportStack;
370+
private final ItemStack data;
370371
private final DestChecker destChecker;
371372
private final Level world;
372373
private double finalScore;
373374
private Direction side;
374375
private List<BlockPos> results = new ArrayList<>();
375376

376-
public Pathfinder(DestChecker checker, InventoryNetwork network, Level world, BlockPos finalNode, BlockPos start, TransporterStack stack) {
377+
public Pathfinder(InventoryNetwork network, Level world, BlockPos finalNode, BlockPos start, TransporterStack stack, ItemStack data, DestChecker checker) {
377378
destChecker = checker;
378379
this.network = network;
379380
this.world = world;
380381
this.finalNode = finalNode;
381382
this.start = start;
382383
transportStack = stack;
384+
this.data = data;
383385
}
384386

385387
public boolean find(Long2ObjectMap<ChunkAccess> chunkMap) {
@@ -469,7 +471,7 @@ private boolean isValidDestination(BlockPos start, @Nullable LogisticalTransport
469471
//Check to make sure that it is the destination
470472
if (startTransporter != null && neighbor.equals(finalNode)) {
471473
BlockEntity neighborTile = WorldUtils.getTileEntity(world, chunkMap, neighbor);
472-
if (destChecker.isValid(world, neighbor, neighborTile, transportStack, direction)) {
474+
if (destChecker.isValid(world, neighbor, neighborTile, transportStack, data, direction)) {
473475
if (startTransporter.canEmitTo(direction) || (finalNode.equals(transportStack.homeLocation) && startTransporter.canConnect(direction))) {
474476
//If it is, and we can emit to it (normal or push mode),
475477
// or it is the home location of the stack (it is returning due to not having been able to get to its destination) and
@@ -494,22 +496,29 @@ private List<BlockPos> reconstructPath(Map<BlockPos, BlockPos> navMap, BlockPos
494496
return path;
495497
}
496498

499+
public boolean hasPath() {
500+
return !results.isEmpty();
501+
}
502+
497503
public List<BlockPos> getPath() {
498-
List<BlockPos> path = new ArrayList<>();
504+
ImmutableList.Builder<BlockPos> path = ImmutableList.builder();
499505
path.add(finalNode);
500506
path.addAll(results);
501-
return path;
507+
return path.build();
508+
}
509+
510+
public double getFinalScore() {
511+
return finalScore;
502512
}
503513

504514
public Direction getSide() {
505515
return side;
506516
}
507517

508-
public static class DestChecker {
518+
@FunctionalInterface
519+
public interface DestChecker {
509520

510-
public boolean isValid(Level level, BlockPos pos, @Nullable BlockEntity tile, TransporterStack stack, Direction side) {
511-
return false;
512-
}
521+
boolean isValid(Level level, BlockPos pos, @Nullable BlockEntity tile, TransporterStack stack, ItemStack data, Direction side);
513522
}
514523
}
515524
}

0 commit comments

Comments
 (0)