Browse Source

Revert "Protect H3 library against integer overflow #92829" (#113773) (#113782)

This library has well defined inputs and outputs so protecting against overflow is not necessary and it introduces a 
significant overhead.
Ignacio Vera 1 year ago
parent
commit
bd5f989f3e

+ 36 - 37
libs/h3/src/main/java/org/elasticsearch/h3/CoordIJK.java

@@ -109,8 +109,8 @@ final class CoordIJK {
      * Find the center point in 2D cartesian coordinates of a hex.
      */
     public Vec2d ijkToHex2d() {
-        final int i = Math.subtractExact(this.i, this.k);
-        final int j = Math.subtractExact(this.j, this.k);
+        final int i = this.i - this.k;
+        final int j = this.j - this.k;
         return new Vec2d(i - 0.5 * j, j * Constants.M_SQRT3_2);
     }
 
@@ -118,8 +118,8 @@ final class CoordIJK {
      * Find the center point in spherical coordinates of a hex on a particular icosahedral face.
      */
     public LatLng ijkToGeo(int face, int res, boolean substrate) {
-        final int i = Math.subtractExact(this.i, this.k);
-        final int j = Math.subtractExact(this.j, this.k);
+        final int i = this.i - this.k;
+        final int j = this.j - this.k;
         return Vec2d.hex2dToGeo(i - 0.5 * j, j * Constants.M_SQRT3_2, face, res, substrate);
     }
 
@@ -132,9 +132,9 @@ final class CoordIJK {
      */
 
     public void ijkAdd(int i, int j, int k) {
-        this.i = Math.addExact(this.i, i);
-        this.j = Math.addExact(this.j, j);
-        this.k = Math.addExact(this.k, k);
+        this.i += i;
+        this.j += j;
+        this.k += k;
     }
 
     /**
@@ -145,9 +145,9 @@ final class CoordIJK {
      * @param k the k coordinate
      */
     public void ijkSub(int i, int j, int k) {
-        this.i = Math.subtractExact(this.i, i);
-        this.j = Math.subtractExact(this.j, j);
-        this.k = Math.subtractExact(this.k, k);
+        this.i -= i;
+        this.j -= j;
+        this.k -= k;
     }
 
     /**
@@ -168,9 +168,9 @@ final class CoordIJK {
         // iVec (3, 0, 1)
         // jVec (1, 3, 0)
         // kVec (0, 1, 3)
-        final int i = Math.addExact(Math.multiplyExact(this.i, 3), this.j);
-        final int j = Math.addExact(Math.multiplyExact(this.j, 3), this.k);
-        final int k = Math.addExact(Math.multiplyExact(this.k, 3), this.i);
+        final int i = this.i * 3 + this.j;
+        final int j = this.j * 3 + this.k;
+        final int k = this.k * 3 + this.i;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -185,9 +185,9 @@ final class CoordIJK {
         // iVec (3, 1, 0)
         // jVec (0, 3, 1)
         // kVec (1, 0, 3)
-        final int i = Math.addExact(Math.multiplyExact(this.i, 3), this.k);
-        final int j = Math.addExact(Math.multiplyExact(this.j, 3), this.i);
-        final int k = Math.addExact(Math.multiplyExact(this.k, 3), this.j);
+        final int i = this.i * 3 + this.k;
+        final int j = this.j * 3 + this.i;
+        final int k = this.k * 3 + this.j;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -203,9 +203,9 @@ final class CoordIJK {
         // iVec (2, 0, 1)
         // jVec (1, 2, 0)
         // kVec (0, 1, 2)
-        final int i = Math.addExact(Math.multiplyExact(this.i, 2), this.j);
-        final int j = Math.addExact(Math.multiplyExact(this.j, 2), this.k);
-        final int k = Math.addExact(Math.multiplyExact(this.k, 2), this.i);
+        final int i = this.i * 2 + this.j;
+        final int j = this.j * 2 + this.k;
+        final int k = this.k * 2 + this.i;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -221,9 +221,9 @@ final class CoordIJK {
         // iVec (2, 1, 0)
         // jVec (0, 2, 1)
         // kVec (1, 0, 2)
-        final int i = Math.addExact(Math.multiplyExact(this.i, 2), this.k);
-        final int j = Math.addExact(Math.multiplyExact(this.j, 2), this.i);
-        final int k = Math.addExact(Math.multiplyExact(this.k, 2), this.j);
+        final int i = this.i * 2 + this.k;
+        final int j = this.j * 2 + this.i;
+        final int k = this.k * 2 + this.j;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -239,9 +239,9 @@ final class CoordIJK {
         // iVec (1, 0, 1)
         // jVec (1, 1, 0)
         // kVec (0, 1, 1)
-        final int i = Math.addExact(this.i, this.j);
-        final int j = Math.addExact(this.j, this.k);
-        final int k = Math.addExact(this.i, this.k);
+        final int i = this.i + this.j;
+        final int j = this.j + this.k;
+        final int k = this.i + this.k;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -256,9 +256,9 @@ final class CoordIJK {
         // iVec (1, 1, 0)
         // jVec (0, 1, 1)
         // kVec (1, 0, 1)
-        final int i = Math.addExact(this.i, this.k);
-        final int j = Math.addExact(this.i, this.j);
-        final int k = Math.addExact(this.j, this.k);
+        final int i = this.i + this.k;
+        final int j = this.i + this.j;
+        final int k = this.j + this.k;
         this.i = i;
         this.j = j;
         this.k = k;
@@ -282,10 +282,10 @@ final class CoordIJK {
      * clockwise aperture 7 grid.
      */
     public void upAp7r() {
-        final int i = Math.subtractExact(this.i, this.k);
-        final int j = Math.subtractExact(this.j, this.k);
-        this.i = (int) Math.round((Math.addExact(Math.multiplyExact(2, i), j)) * M_ONESEVENTH);
-        this.j = (int) Math.round((Math.subtractExact(Math.multiplyExact(3, j), i)) * M_ONESEVENTH);
+        final int i = this.i - this.k;
+        final int j = this.j - this.k;
+        this.i = (int) Math.round((2 * i + j) * M_ONESEVENTH);
+        this.j = (int) Math.round((3 * j - i) * M_ONESEVENTH);
         this.k = 0;
         ijkNormalize();
     }
@@ -296,10 +296,10 @@ final class CoordIJK {
      *
      */
     public void upAp7() {
-        final int i = Math.subtractExact(this.i, this.k);
-        final int j = Math.subtractExact(this.j, this.k);
-        this.i = (int) Math.round((Math.subtractExact(Math.multiplyExact(3, i), j)) * M_ONESEVENTH);
-        this.j = (int) Math.round((Math.addExact(Math.multiplyExact(2, j), i)) * M_ONESEVENTH);
+        final int i = this.i - this.k;
+        final int j = this.j - this.k;
+        this.i = (int) Math.round((3 * i - j) * M_ONESEVENTH);
+        this.j = (int) Math.round((2 * j + i) * M_ONESEVENTH);
         this.k = 0;
         ijkNormalize();
     }
@@ -363,5 +363,4 @@ final class CoordIJK {
             default -> digit;
         };
     }
-
 }

+ 4 - 16
libs/h3/src/main/java/org/elasticsearch/h3/FaceIJK.java

@@ -474,11 +474,7 @@ final class FaceIJK {
                 }
 
                 final int unitScale = unitScaleByCIIres[adjRes] * 3;
-                lastCoord.ijkAdd(
-                    Math.multiplyExact(fijkOrient.translateI, unitScale),
-                    Math.multiplyExact(fijkOrient.translateJ, unitScale),
-                    Math.multiplyExact(fijkOrient.translateK, unitScale)
-                );
+                lastCoord.ijkAdd(fijkOrient.translateI * unitScale, fijkOrient.translateJ * unitScale, fijkOrient.translateK * unitScale);
                 lastCoord.ijkNormalize();
 
                 final Vec2d orig2d1 = lastCoord.ijkToHex2d();
@@ -584,18 +580,10 @@ final class FaceIJK {
                 // to each vertex to translate the vertices to that cell.
                 final int[] vertexLast = verts[lastV];
                 final int[] vertexV = verts[v];
-                scratch2.reset(
-                    Math.addExact(vertexLast[0], this.coord.i),
-                    Math.addExact(vertexLast[1], this.coord.j),
-                    Math.addExact(vertexLast[2], this.coord.k)
-                );
+                scratch2.reset(vertexLast[0] + this.coord.i, vertexLast[1] + this.coord.j, vertexLast[2] + this.coord.k);
                 scratch2.ijkNormalize();
                 final Vec2d orig2d0 = scratch2.ijkToHex2d();
-                scratch2.reset(
-                    Math.addExact(vertexV[0], this.coord.i),
-                    Math.addExact(vertexV[1], this.coord.j),
-                    Math.addExact(vertexV[2], this.coord.k)
-                );
+                scratch2.reset(vertexV[0] + this.coord.i, vertexV[1] + this.coord.j, vertexV[2] + this.coord.k);
                 scratch2.ijkNormalize();
                 final Vec2d orig2d1 = scratch2.ijkToHex2d();
 
@@ -692,7 +680,7 @@ final class FaceIJK {
                 scratch.reset(coord.i, coord.j, coord.k);
                 scratch.downAp7r();
             }
-            scratch.reset(Math.subtractExact(lastI, scratch.i), Math.subtractExact(lastJ, scratch.j), Math.subtractExact(lastK, scratch.k));
+            scratch.reset(lastI - scratch.i, lastJ - scratch.j, lastK - scratch.k);
             scratch.ijkNormalize();
             h = H3Index.H3_set_index_digit(h, r, scratch.unitIjkToDigit());
         }

+ 17 - 16
libs/h3/src/main/java/org/elasticsearch/h3/Vec2d.java

@@ -136,7 +136,7 @@ record Vec2d(
 
         // scale accordingly if this is a substrate grid
         if (substrate) {
-            r /= 3.0;
+            r *= M_ONETHIRD;
             if (H3Index.isResolutionClassIII(res)) {
                 r *= Constants.M_RSQRT7;
             }
@@ -197,17 +197,17 @@ record Vec2d(
                     j = m2;
                 } else {
                     i = m1;
-                    j = Math.incrementExact(m2);
+                    j = m2 + 1;
                 }
             } else {
                 if (r2 < (1.0 - r1)) {
                     j = m2;
                 } else {
-                    j = Math.incrementExact(m2);
+                    j = m2 + 1;
                 }
 
                 if ((1.0 - r1) <= r2 && r2 < (2.0 * r1)) {
-                    i = Math.incrementExact(m1);
+                    i = m1 + 1;
                 } else {
                     i = m1;
                 }
@@ -217,21 +217,21 @@ record Vec2d(
                 if (r2 < (1.0 - r1)) {
                     j = m2;
                 } else {
-                    j = Math.addExact(m2, 1);
+                    j = m2 + 1;
                 }
 
                 if ((2.0 * r1 - 1.0) < r2 && r2 < (1.0 - r1)) {
                     i = m1;
                 } else {
-                    i = Math.incrementExact(m1);
+                    i = m1 + 1;
                 }
             } else {
                 if (r2 < (r1 * 0.5)) {
-                    i = Math.incrementExact(m1);
+                    i = m1 + 1;
                     j = m2;
                 } else {
-                    i = Math.incrementExact(m1);
-                    j = Math.incrementExact(m2);
+                    i = m1 + 1;
+                    j = m2 + 1;
                 }
             }
         }
@@ -242,18 +242,19 @@ record Vec2d(
             if ((j % 2) == 0)  // even
             {
                 final int axisi = j / 2;
-                final int diff = Math.subtractExact(i, axisi);
-                i = Math.subtractExact(i, Math.multiplyExact(2, diff));
+                final int diff = i - axisi;
+                i = i - (2 * diff);
             } else {
-                final int axisi = Math.addExact(j, 1) / 2;
-                final int diff = Math.subtractExact(i, axisi);
-                i = Math.subtractExact(i, Math.addExact(Math.multiplyExact(2, diff), 1));
+                final int axisi = (j + 1) / 2;
+                final int diff = i - axisi;
+                i = i - ((2 * diff) + 1);
             }
         }
 
         if (y < 0.0) {
-            i = Math.subtractExact(i, Math.addExact(Math.multiplyExact(2, j), 1) / 2);
-            j = Math.multiplyExact(-1, j);
+
+            i = i - ((2 * j + 1) / 2);
+            j *= -1;
         }
         final CoordIJK coordIJK = new CoordIJK(i, j, k);
         coordIJK.ijkNormalize();