You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "marin-ma (via GitHub)" <gi...@apache.org> on 2024/08/15 14:05:51 UTC
[PR] [GLUTEN-6863][VL] Pre-alloc and reused compress buffer to avoid OOM in spill [incubator-gluten]
marin-ma opened a new pull request, #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869
During sort-shuffle spill, allocating compressed buffer can trigger another spill and lead to OOM. Because sortBuffer has fixed-size(1M), the maximum compressed buffer size can be computed at the very begining, and the compressed buffer can be pre-allocated and reused for spill.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2330775161
Run Gluten Clickhouse CI
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma merged PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744850568
##########
cpp/velox/shuffle/VeloxSortShuffleWriter.cc:
##########
@@ -106,8 +104,17 @@ arrow::Status VeloxSortShuffleWriter::init() {
options_.partitioning == Partitioning::kSingle,
arrow::Status::Invalid("VeloxSortShuffleWriter doesn't support single partition."));
allocateMinimalArray();
- sortedBuffer_ = facebook::velox::AlignedBuffer::allocate<char>(kSortedBufferSize, veloxPool_.get());
- rawBuffer_ = sortedBuffer_->asMutable<uint8_t>();
+ // In Spark, sortedBuffer_ memory and compressionBuffer_ memory are pre-allocated and counted into executor
+ // memory overhead. To align with Spark, we use arrow::default_memory_pool() to avoid counting these memory in Gluten.
Review Comment:
Do we need to add a function `defaultArrowMemoryPool` to `VeloxMemoryManager` to unify the memory pool usage?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744853464
##########
cpp/velox/tests/VeloxShuffleWriterTest.cc:
##########
@@ -70,24 +70,28 @@ std::vector<ShuffleTestParams> createShuffleTestParams() {
std::vector<int32_t> mergeBufferSizes = {0, 3, 4, 10, 4096};
for (const auto& compression : compressions) {
- for (auto useRadixSort : {true, false}) {
- params.push_back(ShuffleTestParams{
- ShuffleWriterType::kSortShuffle, PartitionWriterType::kLocal, compression, 0, 0, useRadixSort});
+ for (const auto compressionBufferSize : {4, 56, 32 * 1024}) {
Review Comment:
Ok, It's fine to align with Spark behavior here.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2330543117
@jinchengchenghh Do you have further comments? 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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744699322
##########
cpp/core/shuffle/Payload.cc:
##########
@@ -186,45 +186,45 @@ arrow::Result<std::unique_ptr<BlockPayload>> BlockPayload::fromBuffers(
std::vector<std::shared_ptr<arrow::Buffer>> buffers,
const std::vector<bool>* isValidityBuffer,
arrow::MemoryPool* pool,
- arrow::util::Codec* codec) {
+ arrow::util::Codec* codec,
+ std::shared_ptr<arrow::Buffer> compressed) {
if (payloadType == Payload::Type::kCompressed) {
Timer compressionTime;
compressionTime.start();
// Compress.
- // Compressed buffer layout: | buffer1 compressedLength | buffer1 uncompressedLength | buffer1 | ...
- const auto metadataLength = sizeof(int64_t) * 2 * buffers.size();
- int64_t totalCompressedLength =
- std::accumulate(buffers.begin(), buffers.end(), 0LL, [&](auto sum, const auto& buffer) {
- if (!buffer) {
- return sum;
- }
- return sum + codec->MaxCompressedLen(buffer->size(), buffer->data());
- });
- const auto maxCompressedLength = metadataLength + totalCompressedLength;
- ARROW_ASSIGN_OR_RAISE(
- std::shared_ptr<arrow::ResizableBuffer> compressed, arrow::AllocateResizableBuffer(maxCompressedLength, pool));
-
- auto output = compressed->mutable_data();
+ auto maxLength = maxCompressedLength(buffers, codec);
+ std::shared_ptr<arrow::Buffer> compressedBuffer;
+ uint8_t* output;
+ if (compressed) {
+ ARROW_RETURN_IF(
+ compressed->size() < maxLength,
+ arrow::Status::Invalid(
+ "Compressed buffer length < maxCompressedLength. (", compressed->size(), " vs ", maxLength, ")"));
+ output = const_cast<uint8_t*>(compressed->data());
+ } else {
+ ARROW_ASSIGN_OR_RAISE(compressedBuffer, arrow::AllocateResizableBuffer(maxLength, pool));
Review Comment:
We hold the original evicted buffer for uncompressed payload. There are no extra copy.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2330453946
Run Gluten Clickhouse CI
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2322760208
@jinchengchenghh can you take a look?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744692410
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -314,7 +314,7 @@ std::shared_ptr<ColumnarBatch> VeloxHashShuffleReaderDeserializer::next() {
uint32_t numRows;
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, memoryPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
Review Comment:
Before this PR, numRows is set to zero in `BlockPayload::deserialize` once reach EOS. This PR remove this logic and use numRows = 0 to represent a segment of a large row that cannot be compressed within one block.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2330777362
@jinchengchenghh Do you have further comments? 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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1730495905
##########
cpp/velox/shuffle/VeloxSortShuffleWriter.cc:
##########
@@ -106,8 +104,17 @@ arrow::Status VeloxSortShuffleWriter::init() {
options_.partitioning == Partitioning::kSingle,
arrow::Status::Invalid("VeloxSortShuffleWriter doesn't support single partition."));
allocateMinimalArray();
- sortedBuffer_ = facebook::velox::AlignedBuffer::allocate<char>(kSortedBufferSize, veloxPool_.get());
- rawBuffer_ = sortedBuffer_->asMutable<uint8_t>();
+ // In Spark, sortedBuffer_ memory and compressionBuffer_ memory are pre-allocated and counted into executor
+ // memory overhead. To align with Spark, we use arrow::default_memory_pool() to avoid counting these memory in Gluten.
Review Comment:
@FelixYBW arrow::default_memory_pool is used to allocate the sort buffer and compress buffer.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reused compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2291332164
https://github.com/apache/incubator-gluten/issues/6863
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744883251
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/GlutenShuffleUtils.scala:
##########
@@ -91,6 +91,20 @@ object GlutenShuffleUtils {
}
}
+ def getCompressionBufferSize(conf: SparkConf, codec: String): Int = {
+ if ("lz4" == codec) {
+ Math.max(
+ conf.get(IO_COMPRESSION_LZ4_BLOCKSIZE).toInt,
+ GlutenConfig.GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE)
Review Comment:
Per discussion, we will throw exception if `IO_COMPRESSION_LZ4_BLOCKSIZE` < 4. For each serialized row, the row size takes 4 bytes. Therefore 4 bytes is the minimum acceptable compression block size in Gluten.
Note here for the Spark exceptions:
- lz4: spark.io.compression.lz4.blockSize=0
```
Caused by: java.lang.IllegalArgumentException: blockSize must be >= 64, got 0
at net.jpountz.lz4.LZ4BlockOutputStream.compressionLevel(LZ4BlockOutputStream.java:60)
at net.jpountz.lz4.LZ4BlockOutputStream.<init>(LZ4BlockOutputStream.java:101)
at org.apache.spark.io.LZ4CompressionCodec.compressedOutputStream(CompressionCodec.scala:151)
at org.apache.spark.broadcast.TorrentBroadcast$.$anonfun$blockifyObject$2(TorrentBroadcast.scala:361)
at scala.Option.map(Option.scala:230)
at org.apache.spark.broadcast.TorrentBroadcast$.blockifyObject(TorrentBroadcast.scala:361)
at org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:161)
at org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:99)
at org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:38)
at org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:78)
at org.apache.spark.SparkContext.broadcastInternal(SparkContext.scala:1662)
at org.apache.spark.SparkContext.broadcast(SparkContext.scala:1644)
at org.apache.spark.scheduler.DAGScheduler.submitMissingTasks(DAGScheduler.scala:1585)
at org.apache.spark.scheduler.DAGScheduler.submitStage(DAGScheduler.scala:1402)
at org.apache.spark.scheduler.DAGScheduler.handleJobSubmitted(DAGScheduler.scala:1337)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.doOnReceive(DAGScheduler.scala:3003)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:2994)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:2983)
at org.apache.spark.util.EventLoop$$anon$1.run(EventLoop.scala:49)
```
- zstd: spark.io.compression.zstd.bufferSize=0
```
Caused by: java.lang.IllegalArgumentException: Buffer size <= 0
at java.io.BufferedOutputStream.<init>(BufferedOutputStream.java:74)
at org.apache.spark.io.ZStdCompressionCodec.compressedOutputStream(CompressionCodec.scala:237)
at org.apache.spark.broadcast.TorrentBroadcast$.$anonfun$blockifyObject$2(TorrentBroadcast.scala:361)
at scala.Option.map(Option.scala:230)
at org.apache.spark.broadcast.TorrentBroadcast$.blockifyObject(TorrentBroadcast.scala:361)
at org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:161)
at org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:99)
at org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:38)
at org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:78)
at org.apache.spark.SparkContext.broadcastInternal(SparkContext.scala:1662)
at org.apache.spark.SparkContext.broadcast(SparkContext.scala:1644)
at org.apache.spark.scheduler.DAGScheduler.submitMissingTasks(DAGScheduler.scala:1585)
at org.apache.spark.scheduler.DAGScheduler.submitStage(DAGScheduler.scala:1402)
at org.apache.spark.scheduler.DAGScheduler.handleJobSubmitted(DAGScheduler.scala:1337)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.doOnReceive(DAGScheduler.scala:3003)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:2994)
at org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:2983)
at org.apache.spark.util.EventLoop$$anon$1.run(EventLoop.scala:49)
```
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744832332
##########
cpp/velox/tests/VeloxShuffleWriterTest.cc:
##########
@@ -70,24 +70,28 @@ std::vector<ShuffleTestParams> createShuffleTestParams() {
std::vector<int32_t> mergeBufferSizes = {0, 3, 4, 10, 4096};
for (const auto& compression : compressions) {
- for (auto useRadixSort : {true, false}) {
- params.push_back(ShuffleTestParams{
- ShuffleWriterType::kSortShuffle, PartitionWriterType::kLocal, compression, 0, 0, useRadixSort});
+ for (const auto compressionBufferSize : {4, 56, 32 * 1024}) {
Review Comment:
Do we need to support this case or require the compression buffer should be larger than one row size at least, throw exception? I think we should have a check for the minimum config value. @FelixYBW
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744834261
##########
cpp/velox/tests/VeloxShuffleWriterTest.cc:
##########
@@ -70,24 +70,28 @@ std::vector<ShuffleTestParams> createShuffleTestParams() {
std::vector<int32_t> mergeBufferSizes = {0, 3, 4, 10, 4096};
for (const auto& compression : compressions) {
- for (auto useRadixSort : {true, false}) {
- params.push_back(ShuffleTestParams{
- ShuffleWriterType::kSortShuffle, PartitionWriterType::kLocal, compression, 0, 0, useRadixSort});
+ for (const auto compressionBufferSize : {4, 56, 32 * 1024}) {
Review Comment:
Spark doesn't throw exception. It copies the row to a default 32k buffer for compressing.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2309114614
Run Gluten Clickhouse CI
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744695061
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/GlutenShuffleUtils.scala:
##########
@@ -91,6 +91,20 @@ object GlutenShuffleUtils {
}
}
+ def getCompressionBufferSize(conf: SparkConf, codec: String): Int = {
+ if ("lz4" == codec) {
+ Math.max(
+ conf.get(IO_COMPRESSION_LZ4_BLOCKSIZE).toInt,
+ GlutenConfig.GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE)
Review Comment:
64 is not the default value, unless user set `IO_COMPRESSION_LZ4_BLOCKSIZE` to a very small size.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744701057
##########
cpp/core/shuffle/LocalPartitionWriter.cc:
##########
@@ -548,42 +543,14 @@ arrow::Status LocalPartitionWriter::finishSpill(bool close) {
return arrow::Status::OK();
}
-arrow::Status LocalPartitionWriter::evict(
+arrow::Status LocalPartitionWriter::hashEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
Review Comment:
hashEvict need this param to know whether the evict source is a spill or not. If it's spill, the partition writer will write the payload to disk immediately, otherwise it will cache the payload.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744834850
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/GlutenShuffleUtils.scala:
##########
@@ -91,6 +91,20 @@ object GlutenShuffleUtils {
}
}
+ def getCompressionBufferSize(conf: SparkConf, codec: String): Int = {
+ if ("lz4" == codec) {
+ Math.max(
+ conf.get(IO_COMPRESSION_LZ4_BLOCKSIZE).toInt,
+ GlutenConfig.GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE)
Review Comment:
WE could set a more reasonable value, maybe 32 * 1024?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1741362173
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -314,7 +314,7 @@ std::shared_ptr<ColumnarBatch> VeloxHashShuffleReaderDeserializer::next() {
uint32_t numRows;
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, memoryPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
Review Comment:
Why do we have this change?
##########
cpp/core/shuffle/LocalPartitionWriter.cc:
##########
@@ -548,42 +543,14 @@ arrow::Status LocalPartitionWriter::finishSpill(bool close) {
return arrow::Status::OK();
}
-arrow::Status LocalPartitionWriter::evict(
+arrow::Status LocalPartitionWriter::hashEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
Review Comment:
Looks like we don't need `evictType`.
##########
cpp/velox/tests/VeloxShuffleWriterTest.cc:
##########
@@ -70,24 +70,28 @@ std::vector<ShuffleTestParams> createShuffleTestParams() {
std::vector<int32_t> mergeBufferSizes = {0, 3, 4, 10, 4096};
for (const auto& compression : compressions) {
- for (auto useRadixSort : {true, false}) {
- params.push_back(ShuffleTestParams{
- ShuffleWriterType::kSortShuffle, PartitionWriterType::kLocal, compression, 0, 0, useRadixSort});
+ for (const auto compressionBufferSize : {4, 56, 32 * 1024}) {
Review Comment:
Do we have the test for split large row?
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -403,16 +403,43 @@ std::shared_ptr<ColumnarBatch> VeloxSortShuffleReaderDeserializer::next() {
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, arrowPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
reachEos_ = true;
if (cachedRows_ > 0) {
return deserializeToBatch();
}
return nullptr;
}
- auto buffer = std::move(arrowBuffers[0]);
- cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
- cachedRows_ += numRows;
+
+ if (numRows > 0) {
+ auto buffer = std::move(arrowBuffers[0]);
+ cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
+ cachedRows_ += numRows;
+ } else {
+ // For a large row, read all segments.
Review Comment:
Can you explain a bit more? I don't catch the context here.
##########
cpp/core/shuffle/Payload.cc:
##########
@@ -329,6 +329,21 @@ int64_t BlockPayload::rawSize() {
return getBufferSize(buffers_);
}
+int64_t BlockPayload::maxCompressedLength(
Review Comment:
Can we move it to anonymous namespace?
##########
cpp/velox/shuffle/VeloxSortShuffleWriter.cc:
##########
@@ -266,6 +273,7 @@ arrow::Status VeloxSortShuffleWriter::evictAllPartitions() {
}
arrow::Status VeloxSortShuffleWriter::evictPartition(uint32_t partitionId, size_t begin, size_t end) {
+ VELOX_CHECK(begin < end);
Review Comment:
VELOX_DCHECK
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/GlutenShuffleUtils.scala:
##########
@@ -91,6 +91,20 @@ object GlutenShuffleUtils {
}
}
+ def getCompressionBufferSize(conf: SparkConf, codec: String): Int = {
+ if ("lz4" == codec) {
+ Math.max(
+ conf.get(IO_COMPRESSION_LZ4_BLOCKSIZE).toInt,
+ GlutenConfig.GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE)
Review Comment:
Can we support set the config? `GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE`
##########
cpp/core/shuffle/Payload.cc:
##########
@@ -186,45 +186,45 @@ arrow::Result<std::unique_ptr<BlockPayload>> BlockPayload::fromBuffers(
std::vector<std::shared_ptr<arrow::Buffer>> buffers,
const std::vector<bool>* isValidityBuffer,
arrow::MemoryPool* pool,
- arrow::util::Codec* codec) {
+ arrow::util::Codec* codec,
+ std::shared_ptr<arrow::Buffer> compressed) {
if (payloadType == Payload::Type::kCompressed) {
Timer compressionTime;
compressionTime.start();
// Compress.
- // Compressed buffer layout: | buffer1 compressedLength | buffer1 uncompressedLength | buffer1 | ...
- const auto metadataLength = sizeof(int64_t) * 2 * buffers.size();
- int64_t totalCompressedLength =
- std::accumulate(buffers.begin(), buffers.end(), 0LL, [&](auto sum, const auto& buffer) {
- if (!buffer) {
- return sum;
- }
- return sum + codec->MaxCompressedLen(buffer->size(), buffer->data());
- });
- const auto maxCompressedLength = metadataLength + totalCompressedLength;
- ARROW_ASSIGN_OR_RAISE(
- std::shared_ptr<arrow::ResizableBuffer> compressed, arrow::AllocateResizableBuffer(maxCompressedLength, pool));
-
- auto output = compressed->mutable_data();
+ auto maxLength = maxCompressedLength(buffers, codec);
+ std::shared_ptr<arrow::Buffer> compressedBuffer;
+ uint8_t* output;
+ if (compressed) {
+ ARROW_RETURN_IF(
+ compressed->size() < maxLength,
+ arrow::Status::Invalid(
+ "Compressed buffer length < maxCompressedLength. (", compressed->size(), " vs ", maxLength, ")"));
+ output = const_cast<uint8_t*>(compressed->data());
+ } else {
+ ARROW_ASSIGN_OR_RAISE(compressedBuffer, arrow::AllocateResizableBuffer(maxLength, pool));
Review Comment:
Can we reuse the buffer for uncompressed payload type?
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/GlutenShuffleUtils.scala:
##########
@@ -91,6 +91,20 @@ object GlutenShuffleUtils {
}
}
+ def getCompressionBufferSize(conf: SparkConf, codec: String): Int = {
+ if ("lz4" == codec) {
+ Math.max(
+ conf.get(IO_COMPRESSION_LZ4_BLOCKSIZE).toInt,
+ GlutenConfig.GLUTEN_SHUFFLE_COMPRESSION_BUFFER_MIN_SIZE)
Review Comment:
Looks like the default value 64 is much less than other compression kind default value 32 * 1024
##########
cpp/velox/shuffle/VeloxSortShuffleWriter.cc:
##########
@@ -106,8 +104,17 @@ arrow::Status VeloxSortShuffleWriter::init() {
options_.partitioning == Partitioning::kSingle,
arrow::Status::Invalid("VeloxSortShuffleWriter doesn't support single partition."));
allocateMinimalArray();
- sortedBuffer_ = facebook::velox::AlignedBuffer::allocate<char>(kSortedBufferSize, veloxPool_.get());
- rawBuffer_ = sortedBuffer_->asMutable<uint8_t>();
+ // In Spark, sortedBuffer_ memory and compressionBuffer_ memory are pre-allocated and counted into executor
+ // memory overhead. To align with Spark, we use arrow::default_memory_pool() to avoid counting these memory in Gluten.
Review Comment:
@zhztheplayer Can you help look at here? Thanks!
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -403,16 +403,43 @@ std::shared_ptr<ColumnarBatch> VeloxSortShuffleReaderDeserializer::next() {
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, arrowPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
reachEos_ = true;
if (cachedRows_ > 0) {
return deserializeToBatch();
}
return nullptr;
}
- auto buffer = std::move(arrowBuffers[0]);
- cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
- cachedRows_ += numRows;
+
+ if (numRows > 0) {
+ auto buffer = std::move(arrowBuffers[0]);
+ cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
+ cachedRows_ += numRows;
+ } else {
+ // For a large row, read all segments.
Review Comment:
Add some comments here to indicate this cases only occurs in sort buffer writer, and the numRows is 0. Do we have a more friendly way to specify the large row that is splited?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744693857
##########
cpp/velox/tests/VeloxShuffleWriterTest.cc:
##########
@@ -70,24 +70,28 @@ std::vector<ShuffleTestParams> createShuffleTestParams() {
std::vector<int32_t> mergeBufferSizes = {0, 3, 4, 10, 4096};
for (const auto& compression : compressions) {
- for (auto useRadixSort : {true, false}) {
- params.push_back(ShuffleTestParams{
- ShuffleWriterType::kSortShuffle, PartitionWriterType::kLocal, compression, 0, 0, useRadixSort});
+ for (const auto compressionBufferSize : {4, 56, 32 * 1024}) {
Review Comment:
Yes. The condition of splitting a large row is the row size > compressionBufferSize. When compressionBufferSize is 4, most of the rows will be split.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744696440
##########
cpp/core/shuffle/Payload.cc:
##########
@@ -329,6 +329,21 @@ int64_t BlockPayload::rawSize() {
return getBufferSize(buffers_);
}
+int64_t BlockPayload::maxCompressedLength(
Review Comment:
It's a public api for BlockPayload and is used by other 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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744879030
##########
cpp/velox/shuffle/VeloxSortShuffleWriter.cc:
##########
@@ -106,8 +104,17 @@ arrow::Status VeloxSortShuffleWriter::init() {
options_.partitioning == Partitioning::kSingle,
arrow::Status::Invalid("VeloxSortShuffleWriter doesn't support single partition."));
allocateMinimalArray();
- sortedBuffer_ = facebook::velox::AlignedBuffer::allocate<char>(kSortedBufferSize, veloxPool_.get());
- rawBuffer_ = sortedBuffer_->asMutable<uint8_t>();
+ // In Spark, sortedBuffer_ memory and compressionBuffer_ memory are pre-allocated and counted into executor
+ // memory overhead. To align with Spark, we use arrow::default_memory_pool() to avoid counting these memory in Gluten.
Review Comment:
I think the code looks fine now as we don't have a mechanism to count global allocation of Arrow into Spark overhead memory.
In future we may report both Arrow and Velox's global pool usages to one counter which requires for some designs. So far we don't have that.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2330836727
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744862909
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -403,16 +403,44 @@ std::shared_ptr<ColumnarBatch> VeloxSortShuffleReaderDeserializer::next() {
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, arrowPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
reachEos_ = true;
if (cachedRows_ > 0) {
return deserializeToBatch();
}
return nullptr;
}
- auto buffer = std::move(arrowBuffers[0]);
- cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
- cachedRows_ += numRows;
+
+ if (numRows > 0) {
+ auto buffer = std::move(arrowBuffers[0]);
+ cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
+ cachedRows_ += numRows;
+ } else {
+ // numRows = 0 indicates a segment of a large row.
+ std::vector<std::shared_ptr<arrow::Buffer>> buffers;
+ auto rowSize = *reinterpret_cast<RowSizeType*>(const_cast<uint8_t*>(arrowBuffers[0]->data()));
+ RowSizeType bufferSize = arrowBuffers[0]->size();
+ buffers.emplace_back(std::move(arrowBuffers[0]));
+ // Read remaining segments.
+ while (bufferSize < rowSize) {
+ GLUTEN_ASSIGN_OR_THROW(
+ arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, arrowPool_, numRows, decompressTime_));
+ bufferSize += arrowBuffers[0]->size();
+ buffers.emplace_back(std::move(arrowBuffers[0]));
+ }
+ VELOX_CHECK_EQ(bufferSize, rowSize);
+ // Merge all segments.
+ GLUTEN_ASSIGN_OR_THROW(std::shared_ptr<arrow::Buffer> rowBuffer, arrow::AllocateBuffer(rowSize, arrowPool_));
+ RowSizeType bytes = 0;
+ auto* dst = rowBuffer->mutable_data();
+ for (const auto& buffer : buffers) {
+ VELOX_CHECK_NOT_NULL(buffer);
Review Comment:
VELOX_DCHECK, code logic should use DCHECK
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#discussion_r1744860274
##########
cpp/velox/shuffle/VeloxShuffleReader.cc:
##########
@@ -403,16 +403,44 @@ std::shared_ptr<ColumnarBatch> VeloxSortShuffleReaderDeserializer::next() {
GLUTEN_ASSIGN_OR_THROW(
auto arrowBuffers, BlockPayload::deserialize(in_.get(), codec_, arrowPool_, numRows, decompressTime_));
- if (numRows == 0) {
+ if (arrowBuffers.empty()) {
reachEos_ = true;
if (cachedRows_ > 0) {
return deserializeToBatch();
}
return nullptr;
}
- auto buffer = std::move(arrowBuffers[0]);
- cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
- cachedRows_ += numRows;
+
+ if (numRows > 0) {
+ auto buffer = std::move(arrowBuffers[0]);
+ cachedInputs_.emplace_back(numRows, wrapInBufferViewAsOwner(buffer->data(), buffer->size(), buffer));
+ cachedRows_ += numRows;
+ } else {
+ // numRows = 0 indicates a segment of a large row.
Review Comment:
Can we extract the numRows = 0 logic to a function to make code more readable?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2301443099
Run Gluten Clickhouse CI
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2308881226
Run Gluten Clickhouse CI
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6863][VL] Pre-alloc and reuse compress buffer to avoid OOM in spill [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #6869:
URL: https://github.com/apache/incubator-gluten/pull/6869#issuecomment-2308959256
Should we allocate the buffer using global allocator which is counted into overhead memory?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org