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 2020/05/27 03:38:30 UTC

[GitHub] [arrow] liyafan82 opened a new pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

liyafan82 opened a new pull request #7271:
URL: https://github.com/apache/arrow/pull/7271


   In the past, we run integration tests from main methods, and recently, we have changed this to run them by the failsafe plugin.
   
   This is a good change, but it also leads to significant performance degradation. In the past, it took about 10s to run ITTestLargeVector#testLargeDecimalVector, now it takes more than half an hour.
   
   Our investigation shows that the problem was caused by calling HistoricalLog#recordEvent repeatedly. This method is called only when BaseAllocator#DEBUG is enabled. In a unit/integration test, the flag is enabled by default.
   
   We solve the problem with the following steps:
   1. We set system property to disable the BaseAllocator#DEBUG flag.
   2. We change the logic so that the system property takes precedence over the AssertionUtil#isAssertionsEnabled method.
   
   This makes the integration tests as fast as before.


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

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



[GitHub] [arrow] nealrichardson closed pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #7271:
URL: https://github.com/apache/arrow/pull/7271


   


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

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



[GitHub] [arrow] liyafan82 commented on pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7271:
URL: https://github.com/apache/arrow/pull/7271#issuecomment-634411563


   > It looks like this is the time to run integration tests with this patch:
   > 
   > ```
   > Integration / AMD64 Conda Integration Test
   > succeeded 17 hours ago in 21m 9s 
   > ```
   > 
   > from another PR https://github.com/apache/arrow/pull/6425/checks?check_run_id=711561100, it looks like they took
   > 
   > ```
   > Integration / AMD64 Conda Integration Test
   > succeeded 1 hour ago in 26m 54s 
   > ```
   > 
   > It doesn't seem like too much of a speedup, is this what you were expecting or could you point me to some different timings?
   
   @BryanCutler Thanks a lot for your comments. 
   IMO, the integration tests for large buffers/vectors have not been included in "Integration / AMD64 Conda Integration Test"
   It will be a future work item. 


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

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



[GitHub] [arrow] BryanCutler commented on pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on pull request #7271:
URL: https://github.com/apache/arrow/pull/7271#issuecomment-634408467


   It looks like this is the time to run integration tests with this patch:
   ```
   Integration / AMD64 Conda Integration Test
   succeeded 17 hours ago in 21m 9s 
   ```
   from another PR https://github.com/apache/arrow/pull/6425/checks?check_run_id=711561100, it looks like they took
   ```
   Integration / AMD64 Conda Integration Test
   succeeded 1 hour ago in 26m 54s 
   ```
   It doesn't seem like too much of a speedup, is this what you were expecting or could you point me to some different timings?


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

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



[GitHub] [arrow] BryanCutler commented on a change in pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #7271:
URL: https://github.com/apache/arrow/pull/7271#discussion_r430837506



##########
File path: java/memory/src/main/java/org/apache/arrow/memory/BaseAllocator.java
##########
@@ -42,11 +42,21 @@
 
   public static final String DEBUG_ALLOCATOR = "arrow.memory.debug.allocator";
   public static final int DEBUG_LOG_LENGTH = 6;
-  public static final boolean DEBUG = AssertionUtil.isAssertionsEnabled() ||
-      Boolean.parseBoolean(System.getProperty(DEBUG_ALLOCATOR, "false"));
+  public static final boolean DEBUG;
   private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BaseAllocator.class);
   public static final Config DEFAULT_CONFIG = ImmutableConfig.builder().build();
 
+  static {
+    // the system property takes precedence.

Review comment:
       It seems correct to me for the system property to have precedence




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

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



[GitHub] [arrow] github-actions[bot] commented on pull request #7271: ARROW-8940: [Java] Fix the performance degradation of integration tests

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


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


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

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