PHILO-HE commented on code in PR #9103:
URL: https://github.com/apache/incubator-gluten/pull/9103#discussion_r2011171360
##########
cpp/velox/compute/VeloxBackend.cc:
##########
@@ -113,7 +113,8 @@ void VeloxBackend::init(const
std::unordered_map<std::string, std::string>& conf
google::InitGoogleLogging("gluten");
// Allow growing buffer in another task through its memory pool.
- FLAGS_velox_memory_pool_capacity_transfer_across_tasks = true;
+ FLAGS_velox_memory_pool_capacity_transfer_across_tasks =
+ backendConf_->get<int32_t>(kMemoryPoolCapacityTransferAcrossTasks, true);
Review Comment:
int32_t -> bool?
##########
cpp/velox/compute/VeloxRuntime.cc:
##########
@@ -65,6 +70,14 @@ VeloxRuntime::VeloxRuntime(
debugModeEnabled_ = veloxCfg_->get<bool>(kDebugModeEnabled, false);
FLAGS_minloglevel = veloxCfg_->get<uint32_t>(kGlogSeverityLevel,
FLAGS_minloglevel);
FLAGS_v = veloxCfg_->get<uint32_t>(kGlogVerboseLevel, FLAGS_v);
+ FLAGS_velox_exception_user_stacktrace_enabled =
+ veloxCfg_->get<bool>(kEnableUserExceptionStacktrace,
FLAGS_velox_exception_user_stacktrace_enabled);
Review Comment:
In VeloxBackend.cc, one variable holding the default value is used as it
shows below. Better to use the same default variable for consistency? Though
they maybe hold the same bool value currently.
```
backendConf_->get<bool>(kEnableUserExceptionStacktrace,
kEnableUserExceptionStacktraceDefault);
```
Ditto for the following config if applicable.
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -112,6 +112,13 @@ object VeloxConfig {
.timeConf(TimeUnit.MILLISECONDS)
.createWithDefault(TimeUnit.MINUTES.toMillis(60))
+ val COLUMNAR_VELOX_MEMORY_POOL_CAPACITY_TRANSFER_ACROSS_TASKS =
+
buildConf("spark.gluten.sql.columnar.backend.velox.memoryPoolCapacityTransferAcrossTasks")
Review Comment:
The config key name is too long. I know it aligns with the current name
pattern. Maybe, better to simplify all such config names to shorter ones, e.g.,
`spark.gluten.velox.xxx`, in the future`. cc @yikf
--
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]
