You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/05/20 12:38:34 UTC

[GitHub] [arrow] lidavidm commented on a diff in pull request #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+

lidavidm commented on code in PR #13200:
URL: https://github.com/apache/arrow/pull/13200#discussion_r878099832


##########
java/dataset/src/main/java/org/apache/arrow/dataset/jni/DirectReservationListener.java:
##########
@@ -87,11 +87,47 @@ public void unreserve(long size) {
   public long getCurrentDirectMemReservation() {
     try {
       final Class<?> classBits = Class.forName("java.nio.Bits");
-      final Field f = classBits.getDeclaredField("reservedMemory");
+      final Field f = this.getDeclaredFieldBaseOnJDKVersion(classBits, "reservedMemory");
       f.setAccessible(true);
       return ((AtomicLong) f.get(null)).get();
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
   }
+
+  /**
+   * To evaluate method on a class base on JDK version.
+   * @param classBits Object associated with the class with the given string name
+   * @param name Method needed to evaluate
+   * @return

Review Comment:
   ```suggestion
      * Get the given method via reflection, searching for different signatures based on the Java version.
      * @param classBits The java.nio.Bits class.
      * @param name The method being requested.
      * @return The method object.
   ```



##########
java/dataset/src/main/java/org/apache/arrow/dataset/jni/DirectReservationListener.java:
##########
@@ -87,11 +87,47 @@ public void unreserve(long size) {
   public long getCurrentDirectMemReservation() {
     try {
       final Class<?> classBits = Class.forName("java.nio.Bits");
-      final Field f = classBits.getDeclaredField("reservedMemory");
+      final Field f = this.getDeclaredFieldBaseOnJDKVersion(classBits, "reservedMemory");
       f.setAccessible(true);
       return ((AtomicLong) f.get(null)).get();
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
   }
+
+  /**
+   * To evaluate method on a class base on JDK version.
+   * @param classBits Object associated with the class with the given string name
+   * @param name Method needed to evaluate
+   * @return
+   */
+  private Method getDeclaredMethodBaseOnJDKVersion(Class<?> classBits, String name) {
+    try {
+      return classBits.getDeclaredMethod(name, long.class, int.class);
+    } catch (NoSuchMethodException e) {
+      try {
+        return classBits.getDeclaredMethod(name, long.class, long.class);
+      } catch (NoSuchMethodException ex) {
+        throw new AssertionError(ex);
+      }
+    }
+  }
+
+  /**
+   * To evaluate field on a class base on JDK version.
+   * @param classBits Object associated with the class with the given string name
+   * @param name Field needed to evaluate
+   * @return
+   */
+  private Field getDeclaredFieldBaseOnJDKVersion(Class<?> classBits, String name) {

Review Comment:
   This isn't a general-purpose method, so I would just inline it, or at least name it so it's clear it's specifically for the reservedMemory field. (And remove the second parameter in that case.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org