[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r437116925
##
File path: java/memory/src/main/java/io/netty/buffer/NettyArrowBuf.java
##
@@ -404,7 +407,7 @@ protected int _getUnsignedMediumLE(int index) {
this.chk(index, 3);
long addr = this.addr(index);
return PlatformDependent.getByte(addr) & 255 |
-(Short.reverseBytes(PlatformDependent.getShort(addr + 1L)) &
'\u') << 8;
+(Short.reverseBytes(PlatformDependent.getShort(addr + 1L)) & '\u')
<< 8;
Review comment:
(Maybe it is irrelavant to this PR). Here a single call to
PlatformDependent.getShort should be sufficient?
It seems reverseBytes and then shift left amounts to directly getting the
lower bits?
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
##
@@ -78,6 +77,55 @@ public Object run() {
Field addressField = java.nio.Buffer.class.getDeclaredField("address");
addressField.setAccessible(true);
BYTE_BUFFER_ADDRESS_OFFSET = UNSAFE.objectFieldOffset(addressField);
+
+ Constructor directBufferConstructor;
+ long address = -1;
+ final ByteBuffer direct = ByteBuffer.allocateDirect(1);
+ try {
+
+final Object maybeDirectBufferConstructor =
+AccessController.doPrivileged(new PrivilegedAction() {
+ @Override
+ public Object run() {
+try {
+ final Constructor constructor =
+ direct.getClass().getDeclaredConstructor(long.class,
int.class);
+ constructor.setAccessible(true);
+ return constructor;
+} catch (NoSuchMethodException e) {
+ return e;
+} catch (SecurityException e) {
+ return e;
+}
+ }
+});
+
+if (maybeDirectBufferConstructor instanceof Constructor) {
+ address = UNSAFE.allocateMemory(1);
+ // try to use the constructor now
+ try {
+((Constructor)
maybeDirectBufferConstructor).newInstance(address, 1);
+directBufferConstructor = (Constructor)
maybeDirectBufferConstructor;
+logger.debug("direct buffer constructor: available");
+ } catch (InstantiationException e) {
+directBufferConstructor = null;
Review comment:
we should print some logs here?
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
##
@@ -78,6 +77,55 @@ public Object run() {
Field addressField = java.nio.Buffer.class.getDeclaredField("address");
addressField.setAccessible(true);
BYTE_BUFFER_ADDRESS_OFFSET = UNSAFE.objectFieldOffset(addressField);
+
+ Constructor directBufferConstructor;
+ long address = -1;
+ final ByteBuffer direct = ByteBuffer.allocateDirect(1);
+ try {
+
+final Object maybeDirectBufferConstructor =
+AccessController.doPrivileged(new PrivilegedAction() {
+ @Override
+ public Object run() {
+try {
+ final Constructor constructor =
+ direct.getClass().getDeclaredConstructor(long.class,
int.class);
+ constructor.setAccessible(true);
+ return constructor;
+} catch (NoSuchMethodException e) {
+ return e;
Review comment:
It would be helpful for problem diagnostic, if we could print some logs
here?
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -31,19 +28,18 @@
public final long chunkSize;
+ /**
+ * default chunk size from Netty.
+ */
+ private static final long DEFAULT_CHUNK_SIZE = 16777216L;
Review comment:
Is it possible that this value changes from Netty code base?
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r441929412
##
File path: java/memory/src/main/java/io/netty/buffer/NettyArrowBuf.java
##
@@ -404,7 +407,7 @@ protected int _getUnsignedMediumLE(int index) {
this.chk(index, 3);
long addr = this.addr(index);
return PlatformDependent.getByte(addr) & 255 |
-(Short.reverseBytes(PlatformDependent.getShort(addr + 1L)) &
'\u') << 8;
+(Short.reverseBytes(PlatformDependent.getShort(addr + 1L)) & '\u')
<< 8;
Review comment:
Thanks for your effort.
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r441930746
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
##
@@ -78,6 +77,55 @@ public Object run() {
Field addressField = java.nio.Buffer.class.getDeclaredField("address");
addressField.setAccessible(true);
BYTE_BUFFER_ADDRESS_OFFSET = UNSAFE.objectFieldOffset(addressField);
+
+ Constructor directBufferConstructor;
+ long address = -1;
+ final ByteBuffer direct = ByteBuffer.allocateDirect(1);
+ try {
+
+final Object maybeDirectBufferConstructor =
+AccessController.doPrivileged(new PrivilegedAction() {
+ @Override
+ public Object run() {
+try {
+ final Constructor constructor =
+ direct.getClass().getDeclaredConstructor(long.class,
int.class);
+ constructor.setAccessible(true);
+ return constructor;
+} catch (NoSuchMethodException e) {
+ return e;
+} catch (SecurityException e) {
+ return e;
+}
+ }
+});
+
+if (maybeDirectBufferConstructor instanceof Constructor) {
+ address = UNSAFE.allocateMemory(1);
+ // try to use the constructor now
+ try {
+((Constructor)
maybeDirectBufferConstructor).newInstance(address, 1);
+directBufferConstructor = (Constructor)
maybeDirectBufferConstructor;
+logger.debug("direct buffer constructor: available");
+ } catch (InstantiationException e) {
+directBufferConstructor = null;
Review comment:
looks good. thanks.
since the exception handling logic is identical, does it make the code
conciser to use the following semantics?
```
catch (InstantiationException | IllegalAccessException |
InvocationTargetException e) {
...
}
```
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347: URL: https://github.com/apache/arrow/pull/7347#discussion_r441931433 ## File path: java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java ## @@ -31,19 +28,18 @@ public final long chunkSize; + /** + * default chunk size from Netty. + */ + private static final long DEFAULT_CHUNK_SIZE = 16777216L; Review comment: Maybe it is beneficial to add above discussion to the JavaDoc 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: us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446594755
##
File path: java/memory/src/main/java/org/apache/arrow/memory/ArrowBuf.java
##
@@ -227,13 +207,28 @@ public ArrowBuf slice(long index, long length) {
return newBuf;
}
+ /**
+ * Make an nio byte buffery from this arrowbuf.
Review comment:
nit: an -> a
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446595068
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -17,33 +17,107 @@
package org.apache.arrow.memory.rounding;
-import java.lang.reflect.Field;
-
-import org.apache.arrow.memory.NettyAllocationManager;
import org.apache.arrow.memory.util.CommonUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.util.internal.SystemPropertyUtil;
/**
* The default rounding policy. That is, if the requested size is within the
chunk size,
* the rounded size will be the next power of two. Otherwise, the rounded size
* will be identical to the requested size.
*/
public class DefaultRoundingPolicy implements RoundingPolicy {
-
+ private static final Logger logger =
LoggerFactory.getLogger(DefaultRoundingPolicy.class);
public final long chunkSize;
/**
- * The singleton instance.
+ * The variables here and the static block calculates teh DEFAULT_CHUNK_SIZE.
Review comment:
teh -> the
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446617676
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -17,33 +17,107 @@
package org.apache.arrow.memory.rounding;
-import java.lang.reflect.Field;
-
-import org.apache.arrow.memory.NettyAllocationManager;
import org.apache.arrow.memory.util.CommonUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.util.internal.SystemPropertyUtil;
/**
* The default rounding policy. That is, if the requested size is within the
chunk size,
* the rounded size will be the next power of two. Otherwise, the rounded size
* will be identical to the requested size.
*/
public class DefaultRoundingPolicy implements RoundingPolicy {
-
+ private static final Logger logger =
LoggerFactory.getLogger(DefaultRoundingPolicy.class);
public final long chunkSize;
/**
- * The singleton instance.
+ * The variables here and the static block calculates teh DEFAULT_CHUNK_SIZE.
+ *
+ *
+ * It was copied from {@link io.netty.buffer.PooledByteBufAllocator}.
+ *
*/
- public static final DefaultRoundingPolicy INSTANCE = new
DefaultRoundingPolicy();
+ private static final int MIN_PAGE_SIZE = 4096;
+ private static final int MAX_CHUNK_SIZE = (int) (((long) Integer.MAX_VALUE +
1) / 2);
+ private static final long DEFAULT_CHUNK_SIZE;
+ private static final int DEFAULT_PAGE_SIZE;
+ private static final int DEFAULT_MAX_ORDER;
- private DefaultRoundingPolicy() {
+
+ static {
+int defaultPageSize =
SystemPropertyUtil.getInt("org.apache.memory.allocator.pageSize", 8192);
+Throwable pageSizeFallbackCause = null;
try {
- Field field =
NettyAllocationManager.class.getDeclaredField("CHUNK_SIZE");
- field.setAccessible(true);
- chunkSize = (Long) field.get(null);
-} catch (Exception e) {
- throw new RuntimeException("Failed to get chunk size from allocation
manager");
+ validateAndCalculatePageShifts(defaultPageSize);
+} catch (Throwable t) {
+ pageSizeFallbackCause = t;
+ defaultPageSize = 8192;
+}
+DEFAULT_PAGE_SIZE = defaultPageSize;
+
+int defaultMaxOrder =
SystemPropertyUtil.getInt("org.apache.memory.allocator.maxOrder", 11);
+Throwable maxOrderFallbackCause = null;
+try {
+ validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE, defaultMaxOrder);
+} catch (Throwable t) {
+ maxOrderFallbackCause = t;
+ defaultMaxOrder = 11;
+}
+DEFAULT_MAX_ORDER = defaultMaxOrder;
+DEFAULT_CHUNK_SIZE = validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE,
DEFAULT_MAX_ORDER);
+if (logger.isDebugEnabled()) {
+ if (pageSizeFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE, pageSizeFallbackCause);
Review comment:
please remove "DEFAULT_PAGE_SIZE" here
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446617744
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -17,33 +17,107 @@
package org.apache.arrow.memory.rounding;
-import java.lang.reflect.Field;
-
-import org.apache.arrow.memory.NettyAllocationManager;
import org.apache.arrow.memory.util.CommonUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.util.internal.SystemPropertyUtil;
/**
* The default rounding policy. That is, if the requested size is within the
chunk size,
* the rounded size will be the next power of two. Otherwise, the rounded size
* will be identical to the requested size.
*/
public class DefaultRoundingPolicy implements RoundingPolicy {
-
+ private static final Logger logger =
LoggerFactory.getLogger(DefaultRoundingPolicy.class);
public final long chunkSize;
/**
- * The singleton instance.
+ * The variables here and the static block calculates teh DEFAULT_CHUNK_SIZE.
+ *
+ *
+ * It was copied from {@link io.netty.buffer.PooledByteBufAllocator}.
+ *
*/
- public static final DefaultRoundingPolicy INSTANCE = new
DefaultRoundingPolicy();
+ private static final int MIN_PAGE_SIZE = 4096;
+ private static final int MAX_CHUNK_SIZE = (int) (((long) Integer.MAX_VALUE +
1) / 2);
+ private static final long DEFAULT_CHUNK_SIZE;
+ private static final int DEFAULT_PAGE_SIZE;
+ private static final int DEFAULT_MAX_ORDER;
- private DefaultRoundingPolicy() {
+
+ static {
+int defaultPageSize =
SystemPropertyUtil.getInt("org.apache.memory.allocator.pageSize", 8192);
+Throwable pageSizeFallbackCause = null;
try {
- Field field =
NettyAllocationManager.class.getDeclaredField("CHUNK_SIZE");
- field.setAccessible(true);
- chunkSize = (Long) field.get(null);
-} catch (Exception e) {
- throw new RuntimeException("Failed to get chunk size from allocation
manager");
+ validateAndCalculatePageShifts(defaultPageSize);
+} catch (Throwable t) {
+ pageSizeFallbackCause = t;
+ defaultPageSize = 8192;
+}
+DEFAULT_PAGE_SIZE = defaultPageSize;
+
+int defaultMaxOrder =
SystemPropertyUtil.getInt("org.apache.memory.allocator.maxOrder", 11);
+Throwable maxOrderFallbackCause = null;
+try {
+ validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE, defaultMaxOrder);
+} catch (Throwable t) {
+ maxOrderFallbackCause = t;
+ defaultMaxOrder = 11;
+}
+DEFAULT_MAX_ORDER = defaultMaxOrder;
+DEFAULT_CHUNK_SIZE = validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE,
DEFAULT_MAX_ORDER);
+if (logger.isDebugEnabled()) {
+ if (pageSizeFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE, pageSizeFallbackCause);
+ }
+ if (maxOrderFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.maxOrder: {}",
DEFAULT_MAX_ORDER);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.maxOrder: {}",
DEFAULT_MAX_ORDER, maxOrderFallbackCause);
+ }
Review comment:
Please remove "DEFAULT_MAX_ORDER" here
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446626385
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
##
@@ -78,6 +77,55 @@ public Object run() {
Field addressField = java.nio.Buffer.class.getDeclaredField("address");
addressField.setAccessible(true);
BYTE_BUFFER_ADDRESS_OFFSET = UNSAFE.objectFieldOffset(addressField);
+
+ Constructor directBufferConstructor;
+ long address = -1;
+ final ByteBuffer direct = ByteBuffer.allocateDirect(1);
+ try {
+
+final Object maybeDirectBufferConstructor =
+AccessController.doPrivileged(new PrivilegedAction() {
+ @Override
+ public Object run() {
+try {
+ final Constructor constructor =
+ direct.getClass().getDeclaredConstructor(long.class,
int.class);
+ constructor.setAccessible(true);
+ logger.debug("Constructor for direct buffer found and made
accessible");
+ return constructor;
+} catch (NoSuchMethodException e) {
+ logger.debug("Cannot get constructor for direct buffer
allocation", e);
+ return e;
+} catch (SecurityException e) {
+ logger.debug("Cannot get constructor for direct buffer
allocation", e);
+ return e;
+}
+ }
+});
+
+if (maybeDirectBufferConstructor instanceof Constructor) {
+ address = UNSAFE.allocateMemory(1);
+ // try to use the constructor now
+ try {
+((Constructor)
maybeDirectBufferConstructor).newInstance(address, 1);
+directBufferConstructor = (Constructor)
maybeDirectBufferConstructor;
+logger.debug("direct buffer constructor: available");
+ } catch (InstantiationException | IllegalAccessException |
InvocationTargetException e) {
+logger.debug("unable to instantiate a direct buffer via
constructor", e);
Review comment:
here the log level should be warning or error?
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r446639368
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
##
@@ -95,4 +143,26 @@ public static long getByteBufferAddress(ByteBuffer buf) {
private MemoryUtil() {
}
+
+ /**
+ * Create nio byte buffer.
+ */
+ public static ByteBuffer directBuffer(long address, int capacity) {
+if (DIRECT_BUFFER_CONSTRUCTOR != null) {
+ if (capacity < 0) {
+throw new IllegalArgumentException("Capacity is negative, has to be
positive or 0");
+ }
+ try {
+return (ByteBuffer) DIRECT_BUFFER_CONSTRUCTOR.newInstance(address,
capacity);
+ } catch (Throwable cause) {
+// Not expected to ever throw!
+if (cause instanceof Error) {
+ throw (Error) cause;
Review comment:
Is there a special reason for the cast here?
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r447363293
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -17,33 +17,103 @@
package org.apache.arrow.memory.rounding;
-import java.lang.reflect.Field;
-
-import org.apache.arrow.memory.NettyAllocationManager;
import org.apache.arrow.memory.util.CommonUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.util.internal.SystemPropertyUtil;
/**
* The default rounding policy. That is, if the requested size is within the
chunk size,
* the rounded size will be the next power of two. Otherwise, the rounded size
* will be identical to the requested size.
*/
public class DefaultRoundingPolicy implements RoundingPolicy {
-
+ private static final Logger logger =
LoggerFactory.getLogger(DefaultRoundingPolicy.class);
public final long chunkSize;
/**
- * The singleton instance.
+ * The variables here and the static block calculates the DEFAULT_CHUNK_SIZE.
+ *
+ *
+ * It was copied from {@link io.netty.buffer.PooledByteBufAllocator}.
+ *
*/
- public static final DefaultRoundingPolicy INSTANCE = new
DefaultRoundingPolicy();
+ private static final int MIN_PAGE_SIZE = 4096;
+ private static final int MAX_CHUNK_SIZE = (int) (((long) Integer.MAX_VALUE +
1) / 2);
+ private static final long DEFAULT_CHUNK_SIZE;
- private DefaultRoundingPolicy() {
+
+ static {
+int defaultPageSize =
SystemPropertyUtil.getInt("org.apache.memory.allocator.pageSize", 8192);
+Throwable pageSizeFallbackCause = null;
try {
- Field field =
NettyAllocationManager.class.getDeclaredField("CHUNK_SIZE");
- field.setAccessible(true);
- chunkSize = (Long) field.get(null);
-} catch (Exception e) {
- throw new RuntimeException("Failed to get chunk size from allocation
manager");
+ validateAndCalculatePageShifts(defaultPageSize);
+} catch (Throwable t) {
+ pageSizeFallbackCause = t;
+ defaultPageSize = 8192;
+}
+
+int defaultMaxOrder =
SystemPropertyUtil.getInt("org.apache.memory.allocator.maxOrder", 11);
+Throwable maxOrderFallbackCause = null;
+try {
+ validateAndCalculateChunkSize(defaultPageSize, defaultMaxOrder);
+} catch (Throwable t) {
+ maxOrderFallbackCause = t;
+ defaultMaxOrder = 11;
+}
+DEFAULT_CHUNK_SIZE = validateAndCalculateChunkSize(defaultPageSize,
defaultMaxOrder);
+if (logger.isDebugEnabled()) {
+ if (pageSizeFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
defaultPageSize);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
defaultPageSize, pageSizeFallbackCause);
+ }
Review comment:
I think here we only need one parameter here? So it should be
logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
pageSizeFallbackCause);
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r447363481
##
File path:
java/memory/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java
##
@@ -17,33 +17,107 @@
package org.apache.arrow.memory.rounding;
-import java.lang.reflect.Field;
-
-import org.apache.arrow.memory.NettyAllocationManager;
import org.apache.arrow.memory.util.CommonUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.netty.util.internal.SystemPropertyUtil;
/**
* The default rounding policy. That is, if the requested size is within the
chunk size,
* the rounded size will be the next power of two. Otherwise, the rounded size
* will be identical to the requested size.
*/
public class DefaultRoundingPolicy implements RoundingPolicy {
-
+ private static final Logger logger =
LoggerFactory.getLogger(DefaultRoundingPolicy.class);
public final long chunkSize;
/**
- * The singleton instance.
+ * The variables here and the static block calculates teh DEFAULT_CHUNK_SIZE.
+ *
+ *
+ * It was copied from {@link io.netty.buffer.PooledByteBufAllocator}.
+ *
*/
- public static final DefaultRoundingPolicy INSTANCE = new
DefaultRoundingPolicy();
+ private static final int MIN_PAGE_SIZE = 4096;
+ private static final int MAX_CHUNK_SIZE = (int) (((long) Integer.MAX_VALUE +
1) / 2);
+ private static final long DEFAULT_CHUNK_SIZE;
+ private static final int DEFAULT_PAGE_SIZE;
+ private static final int DEFAULT_MAX_ORDER;
- private DefaultRoundingPolicy() {
+
+ static {
+int defaultPageSize =
SystemPropertyUtil.getInt("org.apache.memory.allocator.pageSize", 8192);
+Throwable pageSizeFallbackCause = null;
try {
- Field field =
NettyAllocationManager.class.getDeclaredField("CHUNK_SIZE");
- field.setAccessible(true);
- chunkSize = (Long) field.get(null);
-} catch (Exception e) {
- throw new RuntimeException("Failed to get chunk size from allocation
manager");
+ validateAndCalculatePageShifts(defaultPageSize);
+} catch (Throwable t) {
+ pageSizeFallbackCause = t;
+ defaultPageSize = 8192;
+}
+DEFAULT_PAGE_SIZE = defaultPageSize;
+
+int defaultMaxOrder =
SystemPropertyUtil.getInt("org.apache.memory.allocator.maxOrder", 11);
+Throwable maxOrderFallbackCause = null;
+try {
+ validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE, defaultMaxOrder);
+} catch (Throwable t) {
+ maxOrderFallbackCause = t;
+ defaultMaxOrder = 11;
+}
+DEFAULT_MAX_ORDER = defaultMaxOrder;
+DEFAULT_CHUNK_SIZE = validateAndCalculateChunkSize(DEFAULT_PAGE_SIZE,
DEFAULT_MAX_ORDER);
+if (logger.isDebugEnabled()) {
+ if (pageSizeFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.pageSize: {}",
DEFAULT_PAGE_SIZE, pageSizeFallbackCause);
+ }
+ if (maxOrderFallbackCause == null) {
+logger.debug("-Dorg.apache.memory.allocator.maxOrder: {}",
DEFAULT_MAX_ORDER);
+ } else {
+logger.debug("-Dorg.apache.memory.allocator.maxOrder: {}",
DEFAULT_MAX_ORDER, maxOrderFallbackCause);
+ }
Review comment:
Also here we need only one parameter in the "else" statement.
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:
us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #7347: ARROW-8230: [Java] Remove netty dependency from arrow-memory
liyafan82 commented on a change in pull request #7347:
URL: https://github.com/apache/arrow/pull/7347#discussion_r447363600
##
File path: java/memory/src/main/java/org/apache/arrow/memory/ArrowBuf.java
##
@@ -227,13 +207,25 @@ public ArrowBuf slice(long index, long length) {
return newBuf;
}
+ /**
+ * Make a nio byte buffer from this arrowbuf.
+ */
public ByteBuffer nioBuffer() {
-return asNettyBuffer().nioBuffer();
+return nioBuffer(readerIndex, checkedCastToInt(readableBytes()));
}
+
+ /**
+ * Make an nio byte buffer from this ArrowBuf.
Review comment:
nit: an -> a
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:
us...@infra.apache.org
