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/19 23:00:42 UTC

[GitHub] [arrow] davisusanibar opened a new pull request, #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+

davisusanibar opened a new pull request, #13200:
URL: https://github.com/apache/arrow/pull/13200

   Patch dataset module testing failure with JSE11+


-- 
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


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

Posted by GitBox <gi...@apache.org>.
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


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

Posted by GitBox <gi...@apache.org>.
davisusanibar commented on code in PR #13200:
URL: https://github.com/apache/arrow/pull/13200#discussion_r878351528


##########
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:
   Deleted



##########
java/dataset/src/main/java/org/apache/arrow/dataset/jni/DirectReservationListener.java:
##########
@@ -87,11 +87,40 @@ 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;
+      Field fBaseOnJDKVersion;

Review Comment:
   Changed



-- 
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


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

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13200:
URL: https://github.com/apache/arrow/pull/13200#discussion_r878294468


##########
java/dataset/src/main/java/org/apache/arrow/dataset/jni/DirectReservationListener.java:
##########
@@ -87,11 +87,40 @@ 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;
+      Field fBaseOnJDKVersion;

Review Comment:
   Nit but I would just use `f` and make it non-final here no need to complicate it with extra assignments



-- 
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


[GitHub] [arrow] github-actions[bot] commented on pull request #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13200:
URL: https://github.com/apache/arrow/pull/13200#issuecomment-1132281330

   https://issues.apache.org/jira/browse/ARROW-16537


-- 
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


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

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13200:
URL: https://github.com/apache/arrow/pull/13200#issuecomment-1134975950

   Benchmark runs are scheduled for baseline = d4a7638477769e788030af1e75d55873a92b770b and contender = 442b24b0b9cf11d45245564fa7448f1ca4931ae7. 442b24b0b9cf11d45245564fa7448f1ca4931ae7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a92834fa081f4ae1b7b49dbdaef0fb55...25e0373628e049b6b795873c6b9b8a8b/)
   [Failed :arrow_down:0.51% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/e647d15359254512b0f9e9e46d2cb4be...444c35599f3342f6b7141bdef75d2665/)
   [Failed :arrow_down:0.74% :arrow_up:0.37%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7d30cb024c094c3bb1dd9384dc39d9b3...f8c66284b89742b9b63b5d7a8100dc66/)
   [Finished :arrow_down:0.35% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/37e555f0c87a4526934f82b28bbac9ef...c3a3f3afd4774f6db24174ffb4b69f3e/)
   Buildkite builds:
   [Finished] [`442b24b0` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/808)
   [Failed] [`442b24b0` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/805)
   [Failed] [`442b24b0` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/795)
   [Finished] [`442b24b0` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/811)
   [Finished] [`d4a76384` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/807)
   [Failed] [`d4a76384` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/804)
   [Failed] [`d4a76384` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/794)
   [Finished] [`d4a76384` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/810)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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


[GitHub] [arrow] github-actions[bot] commented on pull request #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13200:
URL: https://github.com/apache/arrow/pull/13200#issuecomment-1132281346

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
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


[GitHub] [arrow] lidavidm closed pull request #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+

Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #13200: ARROW-16537: [Java] Patch dataset module testing failure with JSE11+
URL: https://github.com/apache/arrow/pull/13200


-- 
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


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

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13200:
URL: https://github.com/apache/arrow/pull/13200#issuecomment-1134976102

   ['Python', 'R'] benchmarks have high level of regressions.
   [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7d30cb024c094c3bb1dd9384dc39d9b3...f8c66284b89742b9b63b5d7a8100dc66/)
   


-- 
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