Skip to content

Commit 68e96f2

Browse files
committed
Actually fully fix integer overflow in WatchersSortingList, use long math for the whole thing, we don't care about 32-bit CPUs and the math isn't memory bandwidth bound
1 parent 1cdae10 commit 68e96f2

2 files changed

Lines changed: 36 additions & 30 deletions

File tree

src/main/java/io/github/opencubicchunks/cubicchunks/core/util/WatchersSortingList2D.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class WatchersSortingList2D<T extends BucketSorterEntry & XZAddressable>
5252

5353
private final int intrusiveCollectionId;
5454
private final Supplier<Collection<EntityPlayer>> playersSupplier;
55-
private int[] playerPositions = new int[0];
55+
private long[] playerPositions = new long[0];
5656
private int distributingBucket = 0;
5757

5858
public WatchersSortingList2D(int intrusiveCollectionId, Supplier<Collection<EntityPlayer>> playersSupplier) {
@@ -86,7 +86,7 @@ private void updatePlayerPositions() {
8686
Collection<EntityPlayer> players = playersSupplier.get();
8787
int newSize = players.size() * 2;
8888
if (playerPositions.length != newSize) {
89-
playerPositions = new int[newSize];
89+
playerPositions = new long[newSize];
9090
}
9191
int i = 0;
9292
for (EntityPlayer player : players) {
@@ -253,25 +253,28 @@ private int computeBucketIdx(T element) {
253253
if (playerPositions.length == 0) {
254254
return BUCKET_COUNT - 1;
255255
}
256-
int x = element.getX();
257-
int z = element.getZ();
256+
// Note: don't care about performance on 32-bit JVM, nobody should be using that in 2025+
257+
long x = element.getX();
258+
long z = element.getZ();
258259

259-
int dx = x - playerPositions[0];
260-
int dz = z - playerPositions[1];
261-
int distSqMin = dx*dx + dz*dz;
260+
long dx = x - playerPositions[0];
261+
long dz = z - playerPositions[1];
262+
// Note: for the specific case of 2d we can just about ignore large deltas triggering an overflow -
263+
// only happens when both deltas are Integer.MIN_VALUE which is not a case we support
264+
long distSqMin = dx*dx + dz*dz;
262265
for (int i = 2; i < playerPositions.length; i += 2) {
263266
dx = x - playerPositions[i];
264267
dz = z - playerPositions[i+1];
265-
int distSq = dx*dx + dz*dz;
268+
long distSq = dx*dx + dz*dz;
266269
if (distSq < distSqMin) {
267270
distSqMin = distSq;
268271
}
269272
}
270273
// fast very approximate square root
271-
int log2dist = 32 - Integer.numberOfLeadingZeros(distSqMin);
274+
int log2dist = 64 - Long.numberOfLeadingZeros(distSqMin);
272275
int bitsToCutOff = log2dist >> 1;
273-
int approxDist = distSqMin >> bitsToCutOff;
274-
return Math.min(approxDist, BUCKET_COUNT - 1);
276+
long approxDist = distSqMin >> bitsToCutOff;
277+
return (int) Math.min(approxDist, BUCKET_COUNT - 1);
275278
}
276279

277280
/**

src/main/java/io/github/opencubicchunks/cubicchunks/core/util/WatchersSortingList3D.java

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class WatchersSortingList3D<T extends BucketSorterEntry & XYZAddressable>
5252

5353
private final int intrusiveCollectionId;
5454
private final Supplier<Collection<EntityPlayer>> playersSupplier;
55-
private int[] playerPositions = new int[0];
55+
private long[] playerPositions = new long[0];
5656
private int distributingBucket = 0;
5757

5858
public WatchersSortingList3D(int intrusiveCollectionId, Supplier<Collection<EntityPlayer>> playersSupplier) {
@@ -86,7 +86,7 @@ private void updatePlayerPositions() {
8686
Collection<EntityPlayer> players = playersSupplier.get();
8787
int newSize = players.size() * 3;
8888
if (playerPositions.length != newSize) {
89-
playerPositions = new int[newSize];
89+
playerPositions = new long[newSize];
9090
}
9191
int i = 0;
9292
for (EntityPlayer player : players) {
@@ -253,36 +253,39 @@ private int computeBucketIdx(T element) {
253253
if (playerPositions.length == 0) {
254254
return BUCKET_COUNT - 1;
255255
}
256-
int x = element.getX();
257-
int y = element.getY();
258-
int z = element.getZ();
256+
// Note: don't care about performance on 32-bit JVM, nobody should be using that in 2025+
257+
long x = element.getX();
258+
long y = element.getY();
259+
long z = element.getZ();
259260

260-
int dx = x - playerPositions[0];
261-
int dy = y - playerPositions[1];
262-
int dz = z - playerPositions[2];
263-
long dx2 = (long) dx * (long) dx;
264-
long dy2 = (long) dy * (long) dy;
265-
long dz2 = (long) dz * (long) dz;
261+
long dx = x - playerPositions[0];
262+
long dy = y - playerPositions[1];
263+
long dz = z - playerPositions[2];
264+
long dx2 = dx * dx;
265+
long dy2 = dy * dy;
266+
long dz2 = dz * dz;
266267
long masked = dx2 | dy2 | dz2;
267-
int distSqMin = masked > (long) Integer.MAX_VALUE ? Integer.MAX_VALUE : dx*dx + dy*dy + dz*dz;
268+
// Note: we can't just rely on this being long here, Integer.MAX_VALUE * Integer.MAX_VALUE * 3 still overflows a long
269+
long distSqMin = masked > (long) Integer.MAX_VALUE ? Integer.MAX_VALUE : dx2 + dy2 + dz2;
270+
268271
for (int i = 3; i < playerPositions.length; i += 3) {
269272
dx = x - playerPositions[i];
270273
dy = y - playerPositions[i+1];
271274
dz = z - playerPositions[i+2];
272-
dx2 = (long) dx * (long) dx;
273-
dy2 = (long) dy * (long) dy;
274-
dz2 = (long) dz * (long) dz;
275+
dx2 = dx * dx;
276+
dy2 = dy * dy;
277+
dz2 = dz * dz;
275278
masked = dx2 | dy2 | dz2;
276-
int distSq = masked > (long) Integer.MAX_VALUE ? Integer.MAX_VALUE : dx*dx + dy*dy + dz*dz;
279+
long distSq = masked > (long) Integer.MAX_VALUE ? Integer.MAX_VALUE : dx2 + dy2 + dz2;
277280
if (distSq < distSqMin) {
278281
distSqMin = distSq;
279282
}
280283
}
281284
// fast very approximate square root
282-
int log2dist = 32 - Integer.numberOfLeadingZeros(distSqMin);
285+
int log2dist = 64 - Long.numberOfLeadingZeros(distSqMin);
283286
int bitsToCutOff = log2dist >> 1;
284-
int approxDist = distSqMin >> bitsToCutOff;
285-
int min = Math.min(approxDist, BUCKET_COUNT - 1);
287+
long approxDist = distSqMin >> bitsToCutOff;
288+
int min = (int) Math.min(approxDist, BUCKET_COUNT - 1);
286289
return min;
287290
}
288291

0 commit comments

Comments
 (0)