You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "danepitkin (via GitHub)" <gi...@apache.org> on 2023/10/04 16:15:56 UTC
[PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
danepitkin opened a new pull request, #38016:
URL: https://github.com/apache/arrow/pull/38016
# WIP
### Rationale for this change
The FFM APIs had their 3rd preview release in Java 21. Let's start integrating them with Arrow to see how effective they are. The FFM APIs will presumably be finalized in the next Java LTS release (24).
### What changes are included in this PR?
* Add a new memory module based on java.lang.foreign.
### Are these changes tested?
WIP
### Are there any user-facing changes?
Yes, but it is opt-in for the user.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771376730
> Wow, that looks really promising.
>
> What are the implications of using a preview API? Can the API change in later versions? Does it impact downstream users?
Yes, the APIs can still change, but it is approaching stability now that its been in development for a few years. I plan to mark this module as experimental and only enabled for Java 21+, but I expect it to be finalized for the next Arrow Java LTS (Long Term Support) release Java 24. The benefit of adding it now is that we can trial it and potentially provide late feedback to the JEP implementation if needed.
Adding the module itself won't impact users unless they choose to use it. Arrow Java provides an abstract interface to off-heap memory called `arrow-memory-core`. Users must then select exactly one implementation: sun.misc.Unsafe, netty, and now the experimental FFM (for Java 21+ only). The `arrow-memory-core` interface will need to eventually be refactored to take better advantage of FFM, but I don't plan to do that in this PR.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771393981
> Do you think we'll also ditch our JNI interfaces at some point? I suppose we must keep them until FFM becomes mainstream, but would it be beneficial to use FFM as an alternative on supported setups?
Eventually, yes! I think there is a lot that needs to happen before then. Besides the Java support fiasco holding many users back (Java 8 has extended support until 2030) and needing to wait until Java 24+, the JIT implementations will also most likely require work to improve optimizations of FFM APIs to meet similar performance numbers as the existing alternatives. Long term, this definitely looks like the preferred approach over JNI to me.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1809081702
@github-actions crossbow submit -g java
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "adamkennedy (via GitHub)" <gi...@apache.org>.
adamkennedy commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1809253923
One tidbit passed on from the Trino team was that once we have FFI then we'd also want to figure out how to move past any uses of ByteBuffer in the rest of the code to the FFI classes, as (reportedly) some mistakes were made with ByteBuffer which means it isn't very friendly to inlining and the new stuff is much much faster once inlining has time to kick in.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1346148466
##########
java/memory/memory-ffm/src/main/java/org/apache/arrow/memory/FfmAllocationManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.memory;
+
+import java.lang.foreign.Arena;
+import java.lang.foreign.MemorySegment;
+
+/**
+ * Allocation manager based on java.lang.foreign API.
+ */
+public final class FfmAllocationManager extends AllocationManager {
+
+ private static final ArrowBuf EMPTY = new ArrowBuf(ReferenceManager.NO_OP,
+ null,
+ 0,
+ MemorySegment.NULL.address()
+ );
+
+ public static final AllocationManager.Factory FACTORY = new Factory() {
+ @Override
+ public AllocationManager create(BufferAllocator accountingAllocator, long size) {
+ return new FfmAllocationManager(accountingAllocator, size);
+ }
+
+ @Override
+ public ArrowBuf empty() {
+ return EMPTY;
+ }
+ };
+
+ private final Arena arena;
+
+ private final MemorySegment allocatedMemorySegment;
+
+ private final long allocatedSize;
+
+ private final long allocatedAddress;
+
+ FfmAllocationManager(BufferAllocator accountingAllocator, long requestedSize) {
+ super(accountingAllocator);
+ arena = Arena.ofShared();
Review Comment:
An `Arena` controls the lifecycle of native (off-heap) memory segments. The `Shared` Arena has a bounded lifetime, is explicitly closeable, and is accessible by multiple threads.[1]
[1]https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/foreign/Arena.html
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1747234025
:warning: GitHub issue #37739 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391683714
##########
java/pom.xml:
##########
@@ -1226,6 +1226,17 @@
</build>
</profile>
+ <profile>
+ <!-- Experimental JDK 21+ features -->
+ <id>jdk21+</id>
+ <activation>
+ <jdk>[21,]</jdk>
+ </activation>
+ <modules>
+ <module>memory/memory-ffm</module>
Review Comment:
Is this the best way to enable developers to build arrow-memory-ffm? I haven't come up with a better alternative, but I'm also not a POM expert.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "adamkennedy (via GitHub)" <gi...@apache.org>.
adamkennedy commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391787118
##########
docs/source/java/overview.rst:
##########
@@ -44,6 +44,9 @@ but some modules are JNI bindings to the C++ library.
* - arrow-memory-netty
- Memory management implementation based on Netty.
- Native
+ * - arrow-memory-ffm
Review Comment:
Since the Java API itself is just called "foreign" perhaps just go with their naming and arrow-memory-foreign rather than the acronym?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "wendigo (via GitHub)" <gi...@apache.org>.
wendigo commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-2059910912
Since FFM is GA since JDK 22, is there a plan to move forward with FFM-based allocator for Arrow?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391683714
##########
java/pom.xml:
##########
@@ -1226,6 +1226,17 @@
</build>
</profile>
+ <profile>
+ <!-- Experimental JDK 21+ features -->
+ <id>jdk21+</id>
+ <activation>
+ <jdk>[21,]</jdk>
+ </activation>
+ <modules>
+ <module>memory/memory-ffm</module>
Review Comment:
Is this the best way to enable developers to build arrow-memory-ffm? I haven't come up with a better alternative, but I'm also not a POM expert.
CI does not build this module, but a developer can build from source with `mvn clean install -pl :arrow-memory-ffm --am`
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1393425015
##########
java/memory/memory-core/src/test/java/org/apache/arrow/memory/JavaForeignAllocationManager.java:
##########
@@ -0,0 +1,77 @@
+/*
Review Comment:
This file was added as a hack to test the memory-core module with the FFM APIs. It will be useful for testing later, but can be ignored otherwise.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771381367
Do you think we'll also ditch our JNI interfaces at some point? I suppose we must keep them until FFM becomes mainstream, but would it be beneficial to use FFM as an alternative on supported setups?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771141667
Java newbie here, can you give a pointer to the FFM docs?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771332088
> Added the JEP and docs to the description!
Wow, that looks really promising.
What are the implications of using a preview API? Can the API change in later versions? Does it impact downstream users.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1811537255
I'm going to put this on hold for now. The next steps are to refactor out the usage of `UNSAFE` since it's not accessible in Java 16+. This is a huge task, but would allow us to actually test the memory-core module with FFM APIs. This PR remains an experiment and is not in any mergable form.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1393425015
##########
java/memory/memory-core/src/test/java/org/apache/arrow/memory/JavaForeignAllocationManager.java:
##########
@@ -0,0 +1,77 @@
+/*
Review Comment:
This was added as a hack to test the memory-core module with the FFM APIs. It will be useful for testing later, but can be ignored otherwise.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1809096543
Revision: 0cd79396c440f5d42ea53a5aa357e2d926952275
Submitted crossbow builds: [ursacomputing/crossbow @ actions-0fc32fc5ee](https://github.com/ursacomputing/crossbow/branches/all?query=actions-0fc32fc5ee)
|Task|Status|
|----|------|
|java-jars|[](https://github.com/ursacomputing/crossbow/actions/runs/6855609468/job/18641019146)|
|verify-rc-source-java-linux-almalinux-8-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6855607567/job/18641011208)|
|verify-rc-source-java-linux-conda-latest-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6855608771/job/18641015888)|
|verify-rc-source-java-linux-ubuntu-20.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6855607301/job/18641010411)|
|verify-rc-source-java-linux-ubuntu-22.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6855608211/job/18641013532)|
|verify-rc-source-java-macos-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6855608454/job/18641014664)|
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391681305
##########
java/memory/memory-ffm/src/main/java/org/apache/arrow/memory/FfmAllocationManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.memory;
+
+import java.lang.foreign.Arena;
+import java.lang.foreign.MemorySegment;
+
+/**
+ * Allocation manager based on java.lang.foreign API.
+ */
+public final class FfmAllocationManager extends AllocationManager {
+
+ private static final ArrowBuf EMPTY = new ArrowBuf(ReferenceManager.NO_OP,
+ null,
+ 0,
+ MemorySegment.NULL.address()
+ );
+
+ public static final AllocationManager.Factory FACTORY = new Factory() {
+ @Override
+ public AllocationManager create(BufferAllocator accountingAllocator, long size) {
+ return new FfmAllocationManager(accountingAllocator, size);
+ }
+
+ @Override
+ public ArrowBuf empty() {
+ return EMPTY;
+ }
+ };
+
+ private final Arena arena;
+
+ private final MemorySegment allocatedMemorySegment;
+
+ private final long allocatedSize;
+
+ private final long allocatedAddress;
+
+ FfmAllocationManager(BufferAllocator accountingAllocator, long requestedSize) {
+ super(accountingAllocator);
+ arena = Arena.ofShared();
+ allocatedMemorySegment = arena.allocate(requestedSize, /*byteAlignment*/ 8);
Review Comment:
Note that FFM requires `byteAlignment` to be specified within the allocation manager, while netty/unsafe do not.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1809071447
@github-action crossbow submit -g java
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1771198064
> Java newbie here, can you give a pointer to the FFM docs?
Added the JEP and docs to the descrition!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-1809267796
> One tidbit passed on from the Trino team was that once we have FFI then we'd also want to figure out how to move past any uses of ByteBuffer in the rest of the code to the FFI classes, as (reportedly) some mistakes were made with ByteBuffer which means it isn't very friendly to inlining and the new stuff is much much faster once inlining has time to kick in.
Thank you! I envision this refactor, plus supporting `long` memory addresses, as follow up tasks. I can file those if this PR merges without those features.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391790416
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java:
##########
@@ -130,4 +142,18 @@ private static AllocationManager.Factory getNettyFactory() {
" No DefaultAllocationManager found to instantiate an NettyAllocationManager", e);
}
}
+
+ private static AllocationManager.Factory getFfmFactory() {
+ try {
+ return getFactory("org.apache.arrow.memory.FfmAllocationManager");
+ } catch (RuntimeException e) {
+ if (majorVersion < 21) {
Review Comment:
Yes, good catch. More helpful checks would be nice.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-2501737791
:warning: GitHub issue #37739 **has no components**, please add labels for components.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-2338505140
Closing for now due to staleness.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin closed pull request #38016: GH-37739: [Java] Add experimental arrow-memory-ffm module
URL: https://github.com/apache/arrow/pull/38016
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391789694
##########
docs/source/java/overview.rst:
##########
@@ -44,6 +44,9 @@ but some modules are JNI bindings to the C++ library.
* - arrow-memory-netty
- Memory management implementation based on Netty.
- Native
+ * - arrow-memory-ffm
Review Comment:
SGTM! Good suggestion, will update.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "adamkennedy (via GitHub)" <gi...@apache.org>.
adamkennedy commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391787426
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java:
##########
@@ -55,6 +59,11 @@ public enum AllocationManagerType {
*/
Unsafe,
+ /**
+ * (Experimental) FFM based allocation manager.
+ */
+ FFM,
Review Comment:
Similarly, just use Foreign everywhere?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "adamkennedy (via GitHub)" <gi...@apache.org>.
adamkennedy commented on code in PR #38016:
URL: https://github.com/apache/arrow/pull/38016#discussion_r1391784922
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java:
##########
@@ -130,4 +142,18 @@ private static AllocationManager.Factory getNettyFactory() {
" No DefaultAllocationManager found to instantiate an NettyAllocationManager", e);
}
}
+
+ private static AllocationManager.Factory getFfmFactory() {
+ try {
+ return getFactory("org.apache.arrow.memory.FfmAllocationManager");
+ } catch (RuntimeException e) {
+ if (majorVersion < 21) {
Review Comment:
Would we also have to detect that preview features are actually turned on and the underlying FFM libraries are available in that specific Java 21+ VM instance?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37739: [Java] Add experimental arrow-memory-ffm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #38016:
URL: https://github.com/apache/arrow/pull/38016#issuecomment-2060251886
@wendigo Yes we are planning to.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org