-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-8481: [Java] Provide an allocation manager based on Unsafe API #6956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@BryanCutler I think you had opinions on this from the original PR do you mind taking a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be private? Perhaps the class should be final too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
I have made it package private (to make testing easier), and made the class final.
Please check it. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the default allocator is changing RootAllocator should probably have its defaults[1] changed so that it doesn't generate a default allocator when it wants a netty one.
[1]
| this(configBuilder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignore me...default is still netty
rymurr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also no tests for the new allocator
Thanks for your kind reminder. Test case added. Please check. |
java/memory/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java
Outdated
Show resolved
Hide resolved
This is in response to the discussion in apache#6323 (comment) In this issue, we provide an allocation manager that is capable of allocation large (> 2GB) buffers. In addition, it does not depend on the netty library, which is aligning with the general trend of removing netty dependencies. In the future, we are going to make it the default allocation manager. Closes apache#6956 from liyafan82/fly_0416_unsf Lead-authored-by: liyafan82 <fan_li_ya@foxmail.com> Co-authored-by: emkornfield <emkornfield@gmail.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
This is in response to the discussion in #6323 (comment)
In this issue, we provide an allocation manager that is capable of allocation large (> 2GB) buffers. In addition, it does not depend on the netty library, which is aligning with the general trend of removing netty dependencies. In the future, we are going to make it the default allocation manager.