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/07/08 09:36:58 UTC
[GitHub] [arrow] rymurr opened a new pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
rymurr opened a new pull request #7676:
URL: https://github.com/apache/arrow/pull/7676
As per #7619 (comment) the vector tests should be run for both netty and unsafe allocators
----------------------------------------------------------------
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] rymurr commented on pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663497377
> LGTM, just a couple minor things
Thanks for the review @BryanCutler
I won't rush a small change next time and save you the trouble ;-)
----------------------------------------------------------------
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 a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458732479
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/TestBitVectorHelper.java
##########
@@ -22,50 +22,50 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import org.apache.arrow.memory.ArrowBuf;
-import org.apache.arrow.memory.ReferenceManager;
+import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.junit.Test;
-import io.netty.buffer.PooledByteBufAllocatorL;
import io.netty.util.internal.PlatformDependent;
public class TestBitVectorHelper {
@Test
public void testGetNullCount() throws Exception {
+ BufferAllocator root = new RootAllocator();
Review comment:
I do not find the code for closing the allocator
----------------------------------------------------------------
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 a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458732034
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
Thanks for your clarification.
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663659304
@rymurr looks like this needs a rebase
----------------------------------------------------------------
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 a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458746598
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
When we run tests from IDE, it is also with an unknown allocator, right?
----------------------------------------------------------------
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 a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458591969
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
Can we simply remove this section?
----------------------------------------------------------------
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] rymurr commented on a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458770955
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
yes, thats correct. We would have to set an env variable in the IDE to choose one or the other.
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663629055
Did a quick timing test for this, as expected it doubles the test time for arrow-vector, which is ~10s on my laptop. That seems ok to me.
**Before**
[INFO] Arrow Vectors ...................................... SUCCESS [ 10.986 s]
**After**
[INFO] Arrow Vectors ...................................... SUCCESS [ 20.730 s]
----------------------------------------------------------------
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] rymurr commented on a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r459382974
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
yes, that is quite a bit simpler. Apologies for misunderstanding originally.
I have made that change
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r459596456
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,48 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
+ <id>default-test</id>
+ <phase>test</phase>
+ <configuration>
+ <classpathDependencyExcludes>
+ <classpathDependencyExclude>org.apache.arrow:arrow-memory-unsafe</classpathDependencyExclude>
+ </classpathDependencyExcludes>
+ </configuration>
+ </execution>
+ <execution>
+ <id>run-netty</id>
Review comment:
rename to `run-unsafe` please
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,48 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
I think you can remove the comment now
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663780326
merged to master, thanks @rymurr !
----------------------------------------------------------------
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 closed pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler closed pull request #7676:
URL: https://github.com/apache/arrow/pull/7676
----------------------------------------------------------------
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] rymurr commented on pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663070368
Build fail could be a random network error or related to the parallel build. Have rebased & retriggered.
----------------------------------------------------------------
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] rymurr commented on a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458736961
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/TestBitVectorHelper.java
##########
@@ -22,50 +22,50 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import org.apache.arrow.memory.ArrowBuf;
-import org.apache.arrow.memory.ReferenceManager;
+import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.junit.Test;
-import io.netty.buffer.PooledByteBufAllocatorL;
import io.netty.util.internal.PlatformDependent;
public class TestBitVectorHelper {
@Test
public void testGetNullCount() throws Exception {
+ BufferAllocator root = new RootAllocator();
Review comment:
thanks, agreed. Fixed!
----------------------------------------------------------------
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] rymurr commented on a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r460000196
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,48 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
:+1:
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,48 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
+ <id>default-test</id>
+ <phase>test</phase>
+ <configuration>
+ <classpathDependencyExcludes>
+ <classpathDependencyExclude>org.apache.arrow:arrow-memory-unsafe</classpathDependencyExclude>
+ </classpathDependencyExcludes>
+ </configuration>
+ </execution>
+ <execution>
+ <id>run-netty</id>
Review comment:
:+1:
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458925035
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
would it work to remove the `skip=true` here and add ` <classpathDependencyExclude>org.apache.arrow:arrow-memory-unsafe</classpathDependencyExclude>`, so that the default test runs netty, then leave the additional `run-unsafe` section only?
----------------------------------------------------------------
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 #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-655413717
https://issues.apache.org/jira/browse/ARROW-9371
----------------------------------------------------------------
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] rymurr commented on a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458672541
##########
File path: java/vector/pom.xml
##########
@@ -97,6 +103,58 @@
</resources>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <enableAssertions>true</enableAssertions>
+ <childDelegation>true</childDelegation>
+ <forkCount>${forkCount}</forkCount>
+ <reuseForks>true</reuseForks>
+ <systemPropertyVariables>
+ <java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
+ <io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
+ <user.timezone>UTC</user.timezone>
+ </systemPropertyVariables>
+ <!-- Note: changing the below configuration might increase the max allocation size for a vector
+ which in turn can cause OOM. -->
+ <argLine>-Darrow.vector.max_allocation_bytes=1048576</argLine>
+ </configuration>
+ <executions>
+ <execution>
+ <!-- Skip default tests as they would have two allocation managers on classpath -->
Review comment:
Hey @liyafan82, we can skip this section but we will run the tests 3 times then. One of the three will be with an unknown Allocator (it will be chosen at runtime depending on classpath). I thought it was a bit of a waste.
----------------------------------------------------------------
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] rymurr commented on pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
rymurr commented on pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#issuecomment-663709603
rebase is done, thanks @BryanCutler
I agree re timing, I saw the same numbers. On my laptop the vector compilation time dwarfs the test time so doubling the test time didn't seem too bad.
----------------------------------------------------------------
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 a change in pull request #7676: ARROW-9371: [Java] Run vector tests for both allocators
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7676:
URL: https://github.com/apache/arrow/pull/7676#discussion_r458732733
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/TestBitVectorHelper.java
##########
@@ -22,50 +22,50 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import org.apache.arrow.memory.ArrowBuf;
-import org.apache.arrow.memory.ReferenceManager;
+import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.junit.Test;
-import io.netty.buffer.PooledByteBufAllocatorL;
import io.netty.util.internal.PlatformDependent;
public class TestBitVectorHelper {
@Test
public void testGetNullCount() throws Exception {
+ BufferAllocator root = new RootAllocator();
Review comment:
It would be nice to implement the code by try-with-resource
----------------------------------------------------------------
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