andygrove commented on code in PR #1485:
URL: https://github.com/apache/datafusion-comet/pull/1485#discussion_r1989943308
##########
spark/src/main/java/org/apache/spark/shuffle/comet/CometShuffleMemoryAllocator.java:
##########
@@ -42,36 +42,35 @@ public final class CometShuffleMemoryAllocator extends
CometShuffleMemoryAllocat
/**
* Returns the singleton instance of `CometShuffleMemoryAllocator`. This
method should be used
* instead of the constructor to ensure that only one instance of
`CometShuffleMemoryAllocator` is
- * created. For Spark tests, this returns `CometTestShuffleMemoryAllocator`
which is a test-only
- * allocator that should not be used in production.
+ * created. For Spark tests, this returns
`CometBoundedShuffleMemoryAllocator` which is a
+ * test-only allocator that should not be used in production.
*/
Review Comment:
It is no longer true that this is just for testing.
If the user enables Spark's off-heap memory then we want to use unified
memory management, which means that we use `CometShuffleMemoryAllocator` which
will interact with `TaskMemoryManager` to request memory and this will be
sharing an off-heap pool with Spark.
If the user does not enable off-heap memory then we want to use
`CometBoundedShuffleMemoryAllocator`. The tests also need to default to using
this because off-heap is not enabled when running the Spark SQL tests. The
`COMET_COLUMNAR_SHUFFLE_UNIFIED_MEMORY_ALLOCATOR_IN_TEST` flag allows us to use
unified memory management in the tests (this does assume that off-heap is
enabled). We enable this flag in `CometColumnarShuffleSuite` although I am not
sure why.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
