Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-08-04 Thread via GitHub 


Copilot commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r3713841690


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -120,6 +120,16 @@ ThreadManager* veloxThreadManagerFactory(const 
std::string& kind, std::unique_pt
 void veloxThreadManagerReleaser(ThreadManager* threadManager) {
   delete threadManager;
 }
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);
+  return err == cudaSuccess && count > 0;

Review Comment:
   `cudaGetDeviceCount` can trigger CUDA driver initialization and be 
relatively expensive. If `VeloxBackend::init` can be called multiple times per 
process, consider memoizing the result (e.g., `static std::once_flag` + cached 
bool) so this detection is performed at most once.



##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -194,7 +204,8 @@ void VeloxBackend::init(
 #endif
 
 #ifdef GLUTEN_ENABLE_GPU
-  if (backendConf_->get(kCudfEnabled, kCudfEnabledDefault)) {
+  const auto enableCudf = backendConf_->get(kCudfEnabled, 
kCudfEnabledDefault) && hasCudaRuntimeAndDevice();
+  if (enableCudf) {

Review Comment:
   This change can silently disable cuDF even when `kCudfEnabled` is set, which 
may be confusing operationally. Consider emitting a warning/info log when 
`kCudfEnabled` is true but `hasCudaRuntimeAndDevice()` is false (include the 
CUDA error code/message if available) so misconfiguration or missing 
drivers/devices are diagnosable.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-07-30 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-5133340339

   Run Gluten Clickhouse CI on x86


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-07-30 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-5130562011

   Run Gluten Clickhouse CI on x86


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-07-30 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-5129791880

   Run Gluten Clickhouse CI on x86


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-05-21 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4514378777

   This PR was auto-closed because it has been stalled for 10 days with no 
activity. Please feel free to reopen if it is still valid. Thanks.


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-05-21 Thread via GitHub 


github-actions[bot] closed pull request #11830: [GLUTEN-11828][VL] Use 
immutable gpu config and add cuda runtime detection
URL: https://github.com/apache/gluten/pull/11830


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-05-11 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4426790136

   This PR is stale because it has been open 45 days with no activity. Remove 
stale label or comment or this will be closed in 10 days.


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-27 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r3002694037


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -73,6 +73,17 @@ using namespace facebook;
 namespace gluten {
 
 namespace {
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);

Review Comment:
   added https://github.com/apache/gluten/issues/11844



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996926359


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -73,6 +73,17 @@ using namespace facebook;
 namespace gluten {
 
 namespace {
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);

Review Comment:
   Please create an issue to track this, thanks!



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996830035


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -105,7 +123,15 @@ void VeloxBackend::init(
 
   // Register factories.
   MemoryManager::registerFactory(kVeloxBackendKind, veloxMemoryManagerFactory, 
veloxMemoryManagerReleaser);
-  Runtime::registerFactory(kVeloxBackendKind, veloxRuntimeFactory, 
veloxRuntimeReleaser);
+
+  // Set immutable configurations from backend conf.
+  const bool enableCudf = backendConf_->get(kCudfEnabled, 
kCudfEnabledDefault) && hasCudaRuntimeAndDevice();
+  const bool enableCudfTableScan =
+  enableCudf && backendConf_->get(kCudfEnableTableScan, 
kCudfEnableTableScanDefault);
+  std::unordered_map immutableConf = {

Review Comment:
   Make senses!



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996685256


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -73,6 +73,17 @@ using namespace facebook;
 namespace gluten {
 
 namespace {
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);

Review Comment:
   This is a new problem, we should not require user to install CUDA in CPU 
node, the build pipeline may also need to be updated



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


marin-ma commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996368608


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -105,7 +123,15 @@ void VeloxBackend::init(
 
   // Register factories.
   MemoryManager::registerFactory(kVeloxBackendKind, veloxMemoryManagerFactory, 
veloxMemoryManagerReleaser);
-  Runtime::registerFactory(kVeloxBackendKind, veloxRuntimeFactory, 
veloxRuntimeReleaser);
+
+  // Set immutable configurations from backend conf.
+  const bool enableCudf = backendConf_->get(kCudfEnabled, 
kCudfEnabledDefault) && hasCudaRuntimeAndDevice();
+  const bool enableCudfTableScan =
+  enableCudf && backendConf_->get(kCudfEnableTableScan, 
kCudfEnableTableScanDefault);
+  std::unordered_map immutableConf = {

Review Comment:
   In the current implementation the detection is only executed in backend 
initialisation. If we set it here then the detection will be executed for every 
pipeline, which seems redundant.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996135648


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -81,13 +92,20 @@ void veloxMemoryManagerReleaser(MemoryManager* 
memoryManager) {
   delete memoryManager;
 }
 
-Runtime* veloxRuntimeFactory(
-const std::string& kind,
-MemoryManager* memoryManager,
-const std::unordered_map& sessionConf) {
-  auto* vmm = dynamic_cast(memoryManager);
-  GLUTEN_CHECK(vmm != nullptr, "Not a Velox memory manager");
-  return new VeloxRuntime(kind, vmm, sessionConf);
+Runtime::Factory createVeloxRuntimeFactory(std::unordered_map immutableConf) {
+  return [immutableConf = std::move(immutableConf)](
+ const std::string& kind,
+ MemoryManager* memoryManager,
+ const std::unordered_map& sessionConf) 
-> Runtime* {
+auto* vmm = dynamic_cast(memoryManager);

Review Comment:
   The configuration is not the static config, session level config can make 
user more flexible to decide which query is running on GPU, so I prefer these 
configs still be session config, the session level config can overwrite the 
static config



##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -73,6 +73,17 @@ using namespace facebook;
 namespace gluten {
 
 namespace {
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);

Review Comment:
   Looks like this code would execute on CPU node, but is the function executes 
by header, if not, the cuda library does not exist in CPU node, I'm not sure if 
it can run successfully. If you very it can run well on CPU node without CUDA 
environment, we may need to add a comment on it.
   
   And the common way is to check if nvidia-smi command exists, if exists, we 
can check further.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


marin-ma commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996303322


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -73,6 +73,17 @@ using namespace facebook;
 namespace gluten {
 
 namespace {
+
+bool hasCudaRuntimeAndDevice() {
+#ifdef GLUTEN_ENABLE_GPU
+  int count = 0;
+  cudaError_t err = cudaGetDeviceCount(&count);

Review Comment:
   If we execute the GPU build on a cpu node without CUDA Runtime installed, 
the process will fail early when loading libvelox.so and reporting the cuda 
library is missing.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996295367


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -105,7 +123,15 @@ void VeloxBackend::init(
 
   // Register factories.
   MemoryManager::registerFactory(kVeloxBackendKind, veloxMemoryManagerFactory, 
veloxMemoryManagerReleaser);
-  Runtime::registerFactory(kVeloxBackendKind, veloxRuntimeFactory, 
veloxRuntimeReleaser);
+
+  // Set immutable configurations from backend conf.
+  const bool enableCudf = backendConf_->get(kCudfEnabled, 
kCudfEnabledDefault) && hasCudaRuntimeAndDevice();
+  const bool enableCudfTableScan =
+  enableCudf && backendConf_->get(kCudfEnableTableScan, 
kCudfEnableTableScanDefault);
+  std::unordered_map immutableConf = {

Review Comment:
   We add it here, if detects it is cpu node, we should not set the enable cudf 
to true, could we add a jni call to detect it?
   ```
   val extraConfMap = mutable.Map(GlutenConfig.COLUMNAR_CUDF_ENABLED.key -> 
enableCudf.toString)
   if (!supportsValueStreamDynamicFilter) {
 extraConfMap(VeloxConfig.VALUE_STREAM_DYNAMIC_FILTER_ENABLED.key) = 
"false"
   }
   val extraConf = extraConfMap.asJava
   val transKernel = 
NativePlanEvaluator.create(BackendsApiManager.getBackendName, extraConf)
   ```



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


marin-ma commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4136196326

   Verified using gpu build + spark.gluten.sql.columnar.cudf=true on cpu node. 
@jinchengchenghh Could you help to review? Thanks!


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


jinchengchenghh commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996270046


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -81,13 +92,20 @@ void veloxMemoryManagerReleaser(MemoryManager* 
memoryManager) {
   delete memoryManager;
 }
 
-Runtime* veloxRuntimeFactory(
-const std::string& kind,
-MemoryManager* memoryManager,
-const std::unordered_map& sessionConf) {
-  auto* vmm = dynamic_cast(memoryManager);
-  GLUTEN_CHECK(vmm != nullptr, "Not a Velox memory manager");
-  return new VeloxRuntime(kind, vmm, sessionConf);
+Runtime::Factory createVeloxRuntimeFactory(std::unordered_map immutableConf) {
+  return [immutableConf = std::move(immutableConf)](
+ const std::string& kind,
+ MemoryManager* memoryManager,
+ const std::unordered_map& sessionConf) 
-> Runtime* {
+auto* vmm = dynamic_cast(memoryManager);

Review Comment:
   At this case, we need to set the config in java side to static to forbid 
user setting.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-26 Thread via GitHub 


marin-ma commented on code in PR #11830:
URL: https://github.com/apache/gluten/pull/11830#discussion_r2996252135


##
cpp/velox/compute/VeloxBackend.cc:
##
@@ -81,13 +92,20 @@ void veloxMemoryManagerReleaser(MemoryManager* 
memoryManager) {
   delete memoryManager;
 }
 
-Runtime* veloxRuntimeFactory(
-const std::string& kind,
-MemoryManager* memoryManager,
-const std::unordered_map& sessionConf) {
-  auto* vmm = dynamic_cast(memoryManager);
-  GLUTEN_CHECK(vmm != nullptr, "Not a Velox memory manager");
-  return new VeloxRuntime(kind, vmm, sessionConf);
+Runtime::Factory createVeloxRuntimeFactory(std::unordered_map immutableConf) {
+  return [immutableConf = std::move(immutableConf)](
+ const std::string& kind,
+ MemoryManager* memoryManager,
+ const std::unordered_map& sessionConf) 
-> Runtime* {
+auto* vmm = dynamic_cast(memoryManager);

Review Comment:
   The cudf initialisation is only executed once in `VeloxBackend::init`. If 
the cudf related configurations are set at runtime by user, the initialisation 
code will be not be executed.



-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-25 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4127918042

   Run Gluten Clickhouse CI on x86


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-25 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4127867621

   Run Gluten Clickhouse CI on x86


-- 
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]

Re: [PR] [GLUTEN-11828][VL] Use immutable gpu config and add cuda runtime detection [gluten]

2026-03-25 Thread via GitHub 


github-actions[bot] commented on PR #11830:
URL: https://github.com/apache/gluten/pull/11830#issuecomment-4127823827

   Run Gluten Clickhouse CI on x86


-- 
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]