From a69cefd8d42b29c36a92c57ad1284aa25fb9b3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:39:39 +0300 Subject: [PATCH 1/4] Fixing BoundingBoxes caching --- .../server/collision/BoundingBox.java | 155 ++++++++++++------ 1 file changed, 101 insertions(+), 54 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index 447d37430..5a1bc5b20 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -1,5 +1,6 @@ package net.minestom.server.collision; +import com.google.common.collect.ImmutableList; import net.minestom.server.coordinate.Point; import net.minestom.server.coordinate.Pos; import net.minestom.server.coordinate.Vec; @@ -7,8 +8,9 @@ import net.minestom.server.entity.Entity; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collections; import java.util.List; -import java.util.function.Supplier; +import java.util.concurrent.atomic.AtomicReference; /** * See https://wiki.vg/Entity_metadata#Mobs_2 @@ -18,13 +20,72 @@ public class BoundingBox { private final Entity entity; private final double x, y, z; - private volatile Pos lastPosition; - private List bottomFace; - private List topFace; - private List leftFace; - private List rightFace; - private List frontFace; - private List backFace; + private final CachedFace bottomFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace topFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + ); + } + }; + private final CachedFace leftFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace rightFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace frontFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()) + ); + } + }; + private final CachedFace backFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + ); + } + }; /** * Creates a {@link BoundingBox} linked to an {@link Entity} and with a specific size. @@ -320,12 +381,7 @@ public class BoundingBox { * @return the points at the bottom of the {@link BoundingBox} */ public @NotNull List getBottomFace() { - this.bottomFace = get(bottomFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()))); - return bottomFace; + return bottomFace.get(); } /** @@ -334,12 +390,7 @@ public class BoundingBox { * @return the points at the top of the {@link BoundingBox} */ public @NotNull List getTopFace() { - this.topFace = get(topFace, () -> - List.of(new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()))); - return topFace; + return topFace.get(); } /** @@ -348,12 +399,7 @@ public class BoundingBox { * @return the points on the left face of the {@link BoundingBox} */ public @NotNull List getLeftFace() { - this.leftFace = get(leftFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()))); - return leftFace; + return leftFace.get(); } /** @@ -362,12 +408,7 @@ public class BoundingBox { * @return the points on the right face of the {@link BoundingBox} */ public @NotNull List getRightFace() { - this.rightFace = get(rightFace, () -> - List.of(new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()))); - return rightFace; + return rightFace.get(); } /** @@ -376,12 +417,7 @@ public class BoundingBox { * @return the points at the front of the {@link BoundingBox} */ public @NotNull List getFrontFace() { - this.frontFace = get(frontFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()))); - return frontFace; + return frontFace.get(); } /** @@ -390,12 +426,7 @@ public class BoundingBox { * @return the points at the back of the {@link BoundingBox} */ public @NotNull List getBackFace() { - this.backFace = get(backFace, () -> List.of( - new Vec(getMinX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()))); - return backFace; + return backFace.get(); } @Override @@ -410,18 +441,34 @@ public class BoundingBox { return result; } - private @NotNull List get(@Nullable List face, Supplier> vecSupplier) { - final var lastPos = this.lastPosition; - final var entityPos = entity.getPosition(); - if (face != null && lastPos != null && lastPos.samePoint(entityPos)) { - return face; - } - this.lastPosition = entityPos; - return vecSupplier.get(); - } - private enum Axis { X, Y, Z } + private abstract class CachedFace { + + private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + + abstract @NotNull List producePoints(); + + @NotNull List get() { + return reference.updateAndGet(value -> { + Pos entityPosition = entity.getPosition(); + if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { + value.lastPosition = entityPosition; + value.points = producePoints(); + } + return value; + }).points; + } + + } + + private static class PositionedPoints { + + private @Nullable Pos lastPosition; + private @NotNull List points = Collections.emptyList(); + + } + } From 34ec59dc68d8955b195d9eadbedd2ffc0e4c213e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:52:38 +0300 Subject: [PATCH 2/4] ImmutableList.of() to List.of() --- .../net/minestom/server/collision/BoundingBox.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index 5a1bc5b20..d603979f0 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -1,6 +1,5 @@ package net.minestom.server.collision; -import com.google.common.collect.ImmutableList; import net.minestom.server.coordinate.Point; import net.minestom.server.coordinate.Pos; import net.minestom.server.coordinate.Vec; @@ -23,7 +22,7 @@ public class BoundingBox { private final CachedFace bottomFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMaxZ()), @@ -34,7 +33,7 @@ public class BoundingBox { private final CachedFace topFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), @@ -45,7 +44,7 @@ public class BoundingBox { private final CachedFace leftFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMinX(), getMaxY(), getMinZ()), new Vec(getMinX(), getMaxY(), getMaxZ()), @@ -56,7 +55,7 @@ public class BoundingBox { private final CachedFace rightFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), @@ -67,7 +66,7 @@ public class BoundingBox { private final CachedFace frontFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), @@ -78,7 +77,7 @@ public class BoundingBox { private final CachedFace backFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMaxZ()), new Vec(getMaxX(), getMinY(), getMaxZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), From 32b33d6bf970d92d63d8e90a5bfb19519ac18837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:55:24 +0300 Subject: [PATCH 3/4] Code review --- .../server/collision/BoundingBox.java | 112 +++++++----------- 1 file changed, 43 insertions(+), 69 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index d603979f0..dfe9370fb 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -10,6 +10,7 @@ import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; /** * See https://wiki.vg/Entity_metadata#Mobs_2 @@ -19,72 +20,42 @@ public class BoundingBox { private final Entity entity; private final double x, y, z; - private final CachedFace bottomFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace topFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()) - ); - } - }; - private final CachedFace leftFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace rightFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace frontFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()) - ); - } - }; - private final CachedFace backFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()) - ); - } - }; + private final CachedFace bottomFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + )); + private final CachedFace topFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + )); + private final CachedFace leftFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + )); + private final CachedFace rightFace = new CachedFace(() -> List.of( + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()) + )); + private final CachedFace frontFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()) + )); + private final CachedFace backFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + )); /** * Creates a {@link BoundingBox} linked to an {@link Entity} and with a specific size. @@ -444,18 +415,21 @@ public class BoundingBox { X, Y, Z } - private abstract class CachedFace { + private class CachedFace { private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + private final Supplier<@NotNull List> faceProducer; - abstract @NotNull List producePoints(); + private CachedFace(Supplier<@NotNull List> faceProducer) { + this.faceProducer = faceProducer; + } @NotNull List get() { return reference.updateAndGet(value -> { Pos entityPosition = entity.getPosition(); if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { value.lastPosition = entityPosition; - value.points = producePoints(); + value.points = faceProducer.get(); } return value; }).points; From 224345853e2c665644e69073145df881a1239a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 16:16:43 +0300 Subject: [PATCH 4/4] BoundingBox#PositionedPoints is immutable now --- .../minestom/server/collision/BoundingBox.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index dfe9370fb..7aca98339 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -417,7 +417,7 @@ public class BoundingBox { private class CachedFace { - private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + private final AtomicReference<@Nullable PositionedPoints> reference = new AtomicReference<>(null); private final Supplier<@NotNull List> faceProducer; private CachedFace(Supplier<@NotNull List> faceProducer) { @@ -425,11 +425,11 @@ public class BoundingBox { } @NotNull List get() { + //noinspection ConstantConditions return reference.updateAndGet(value -> { Pos entityPosition = entity.getPosition(); - if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { - value.lastPosition = entityPosition; - value.points = faceProducer.get(); + if (value == null || !value.lastPosition.samePoint(entityPosition)) { + return new PositionedPoints(entityPosition, faceProducer.get()); } return value; }).points; @@ -439,8 +439,13 @@ public class BoundingBox { private static class PositionedPoints { - private @Nullable Pos lastPosition; - private @NotNull List points = Collections.emptyList(); + private final @NotNull Pos lastPosition; + private final @NotNull List points; + + private PositionedPoints(@NotNull Pos lastPosition, @NotNull List points) { + this.lastPosition = lastPosition; + this.points = points; + } }