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/06/09 16:32:00 UTC
[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)) & '\uffff') << 8;
+ (Short.reverseBytes(PlatformDependent.getShort(addr + 1L)) & '\uffff') << 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<Object>() {
+ @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<Object>() {
+ @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:
users@infra.apache.org