Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ class VeloxCelebornColumnarShuffleWriter[K, V](
val columnarBatchHandle =
ColumnarBatches.getNativeHandle(BackendsApiManager.getBackendName, cb)
val startTime = System.nanoTime()
shuffleWriterJniWrapper.write(
val bytesWritten = shuffleWriterJniWrapper.write(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think VeloxUniffleColumnarShuffleWriter also needs to be updated. It's using the same native partition writer as celeborn.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uniffle does not call incBytesWritten and the logic of metrics collecting differs with local and celeborn, it's better to let related developer to change it.

nativeShuffleWriter,
cb.numRows,
columnarBatchHandle,
availableOffHeapPerTask())
dep.metrics("shuffleWallTime").add(System.nanoTime() - startTime)
dep.metrics("numInputRows").add(cb.numRows)
dep.metrics("inputBatches").add(1)
writeMetrics.incBytesWritten(bytesWritten)
// This metric is important, AQE use it to decide if EliminateLimit
writeMetrics.incRecordsWritten(cb.numRows())
}
Expand Down Expand Up @@ -120,7 +121,7 @@ class VeloxCelebornColumnarShuffleWriter[K, V](
dep.metrics("c2rTime").add(splitResult.getC2RTime)
}
dep.metrics("dataSize").add(splitResult.getRawPartitionLengths.sum)
writeMetrics.incBytesWritten(splitResult.getTotalBytesWritten)
writeMetrics.incBytesWritten(splitResult.getBytesWritten)
writeMetrics.incWriteTime(splitResult.getTotalWriteTime + splitResult.getTotalPushTime)

partitionLengths = splitResult.getPartitionLengths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,15 @@ class ColumnarShuffleWriter[K, V](
val columnarBatchHandle =
ColumnarBatches.getNativeHandle(BackendsApiManager.getBackendName, cb)
val startTime = System.nanoTime()
shuffleWriterJniWrapper.write(
val bytesWritten = shuffleWriterJniWrapper.write(
nativeShuffleWriter,
rows,
columnarBatchHandle,
availableOffHeapPerTask())
dep.metrics("shuffleWallTime").add(System.nanoTime() - startTime)
dep.metrics("numInputRows").add(rows)
dep.metrics("inputBatches").add(1)
writeMetrics.incBytesWritten(bytesWritten)
Comment thread
Yohahaha marked this conversation as resolved.
// This metric is important, AQE use it to decide if EliminateLimit
writeMetrics.incRecordsWritten(rows)
}
Expand Down Expand Up @@ -256,7 +257,7 @@ class ColumnarShuffleWriter[K, V](
dep.metrics("dataSize").add(splitResult.getRawPartitionLengths.sum)
dep.metrics("compressTime").add(splitResult.getTotalCompressTime)
dep.metrics("peakBytes").add(splitResult.getPeakBytes)
writeMetrics.incBytesWritten(splitResult.getTotalBytesWritten)
writeMetrics.incBytesWritten(splitResult.getBytesWritten)
writeMetrics.incWriteTime(splitResult.getTotalWriteTime + splitResult.getTotalSpillTime)
taskContext.taskMetrics().incMemoryBytesSpilled(splitResult.getBytesToEvict)
taskContext.taskMetrics().incDiskBytesSpilled(splitResult.getTotalBytesSpilled)
Expand Down
5 changes: 2 additions & 3 deletions cpp/core/jni/JniWrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1038,9 +1038,8 @@ JNIEXPORT jlong JNICALL Java_org_apache_gluten_vectorized_ShuffleWriterJniWrappe

// The column batch maybe VeloxColumnBatch or ArrowCStructColumnarBatch(FallbackRangeShuffleWriter)
auto batch = ObjectStore::retrieve<ColumnarBatch>(batchHandle);
auto numBytes = batch->numBytes();
arrowAssertOkOrThrow(shuffleWriter->write(batch, memLimit), "Native write: shuffle writer failed");
return numBytes;
return shuffleWriter->bytesWritten();
JNI_METHOD_END(kInvalidObjectHandle)
}

Expand Down Expand Up @@ -1076,7 +1075,7 @@ JNIEXPORT jobject JNICALL Java_org_apache_gluten_vectorized_ShuffleWriterJniWrap
shuffleWriter->totalCompressTime(),
shuffleWriter->totalSortTime(),
shuffleWriter->totalC2RTime(),
shuffleWriter->totalBytesWritten(),
shuffleWriter->bytesWritten(),
shuffleWriter->totalBytesEvicted(),
shuffleWriter->totalBytesToEvict(),
shuffleWriter->peakBytesAllocated(),
Expand Down
8 changes: 5 additions & 3 deletions cpp/core/shuffle/LocalPartitionWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ arrow::Status LocalPartitionWriter::writeCachedPayloads(uint32_t partitionId, ar
return arrow::Status::OK();
}

arrow::Status LocalPartitionWriter::stop(ShuffleWriterMetrics* metrics) {
arrow::Status LocalPartitionWriter::stop(ShuffleWriterMetrics* metrics, int64_t& evictBytes) {
if (stopped_) {
return arrow::Status::OK();
}
Expand Down Expand Up @@ -645,6 +645,7 @@ arrow::Status LocalPartitionWriter::stop(ShuffleWriterMetrics* metrics) {
}
}
ARROW_ASSIGN_OR_RAISE(totalBytesWritten_, dataFileOs_->Tell());
evictBytes += totalBytesWritten_;

// Close Final file. Clear buffered resources.
RETURN_NOT_OK(clearResource());
Expand Down Expand Up @@ -710,7 +711,8 @@ arrow::Status LocalPartitionWriter::hashEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
bool reuseBuffers) {
bool reuseBuffers,
int64_t& evictBytes) {
rawPartitionLengths_[partitionId] += inMemoryPayload->rawSize();

if (evictType == Evict::kSpill) {
Expand Down Expand Up @@ -754,7 +756,7 @@ arrow::Status LocalPartitionWriter::hashEvict(
}

arrow::Status
LocalPartitionWriter::sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal) {
LocalPartitionWriter::sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal, int64_t& evictBytes) {
rawPartitionLengths_[partitionId] += inMemoryPayload->rawSize();

if (lastEvictPid_ != -1 && (partitionId < lastEvictPid_ || (isFinal && !dataFileOs_))) {
Expand Down
15 changes: 10 additions & 5 deletions cpp/core/shuffle/LocalPartitionWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,18 @@ class LocalPartitionWriter : public PartitionWriter {
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
bool reuseBuffers) override;
bool reuseBuffers,
int64_t& evictBytes) override;

arrow::Status sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal)
override;
arrow::Status sortEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
bool isFinal,
int64_t& evictBytes) override;

// This code path is not used by LocalPartitionWriter, Not implement it by default.
arrow::Status evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop) override {
arrow::Status evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop, int64_t& evictBytes)
override {
return arrow::Status::NotImplemented("Invalid code path for local shuffle writer.");
}

Expand All @@ -68,7 +73,7 @@ class LocalPartitionWriter : public PartitionWriter {
/// If spill is triggered by 2.c, cached payloads of the remaining unmerged partitions will be spilled.
/// In both cases, if the cached payload size doesn't free enough memory,
/// it will shrink partition buffers to free more memory.
arrow::Status stop(ShuffleWriterMetrics* metrics) override;
arrow::Status stop(ShuffleWriterMetrics* metrics, int64_t& evictBytes) override;

// Spill source:
// 1. Other op.
Expand Down
15 changes: 10 additions & 5 deletions cpp/core/shuffle/PartitionWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,24 @@ class PartitionWriter : public Reclaimable {

~PartitionWriter() override = default;

virtual arrow::Status stop(ShuffleWriterMetrics* metrics) = 0;
virtual arrow::Status stop(ShuffleWriterMetrics* metrics, int64_t& evictBytes) = 0;

/// Evict buffers for `partitionId` partition.
virtual arrow::Status hashEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
bool reuseBuffers) = 0;
bool reuseBuffers,
int64_t& evictBytes) = 0;

virtual arrow::Status
sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal) = 0;
virtual arrow::Status sortEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
bool isFinal,
int64_t& evictBytes) = 0;

virtual arrow::Status evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop) = 0;
virtual arrow::Status
evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop, int64_t& evictBytes) = 0;

uint64_t cachedPayloadSize() {
return payloadPool_->bytes_allocated();
Expand Down
4 changes: 2 additions & 2 deletions cpp/core/shuffle/ShuffleWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ int32_t ShuffleWriter::numPartitions() const {
return numPartitions_;
}

int64_t ShuffleWriter::totalBytesWritten() const {
return metrics_.totalBytesWritten;
int64_t ShuffleWriter::bytesWritten() const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simply modify the signature ofShuffleWriter::write and ShuffleWriter::stop to return the bytes evicted as Result<int64_t>? And seems like this value is not updated and used for local shuffle write, please add some explanation that only rss shuffle write returns the evicted bytes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simply modify the signature ofShuffleWriter::write and ShuffleWriter::stop to return the bytes evicted as Result<int64_t>

The write api can be easily modified, but the stop api is not, stop api collect and return whole shuffle metrics of task, I prefer do refactor work for api change in another pr and make current pr as simple as possible.

return writtenBytes_;
}

int64_t ShuffleWriter::totalBytesEvicted() const {
Expand Down
3 changes: 2 additions & 1 deletion cpp/core/shuffle/ShuffleWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ShuffleWriter : public Reclaimable {

int32_t numPartitions() const;

int64_t totalBytesWritten() const;
int64_t bytesWritten() const;

int64_t totalBytesEvicted() const;

Expand Down Expand Up @@ -76,6 +76,7 @@ class ShuffleWriter : public Reclaimable {
Partitioning partitioning_;

ShuffleWriterMetrics metrics_{};
int64_t writtenBytes_{0};
};

} // namespace gluten
25 changes: 16 additions & 9 deletions cpp/core/shuffle/rss/RssPartitionWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void RssPartitionWriter::init() {
rawPartitionLengths_.resize(numPartitions_, 0);
}

arrow::Status RssPartitionWriter::stop(ShuffleWriterMetrics* metrics) {
arrow::Status RssPartitionWriter::stop(ShuffleWriterMetrics* metrics, int64_t& /* evictBytes */) {
spillTime_ -= compressTime_;
rssClient_->stop();

Expand All @@ -53,12 +53,13 @@ arrow::Status RssPartitionWriter::hashEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
bool reuseBuffers) {
return doEvict(partitionId, std::move(inMemoryPayload));
bool reuseBuffers,
int64_t& evictBytes) {
return doEvict(partitionId, std::move(inMemoryPayload), evictBytes);
}

arrow::Status
RssPartitionWriter::sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal) {
RssPartitionWriter::sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal, int64_t& evictBytes) {
ScopedTimer timer(&spillTime_);
rawPartitionLengths_[partitionId] += inMemoryPayload->rawSize();
if (shouldInitializeOs_) {
Expand All @@ -85,22 +86,26 @@ RssPartitionWriter::sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayl
compressTime_ += compressedOs_->compressTime();
}
ARROW_ASSIGN_OR_RAISE(const auto buffer, rssOs_->Finish());
bytesEvicted_[partitionId] += rssClient_->pushPartitionData(partitionId, buffer->data_as<char>(), buffer->size());
auto delta = rssClient_->pushPartitionData(partitionId, buffer->data_as<char>(), buffer->size());
bytesEvicted_[partitionId] += delta;
evictBytes += delta;
shouldInitializeOs_ = true;
}

return arrow::Status::OK();
}

arrow::Status RssPartitionWriter::evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool) {
arrow::Status RssPartitionWriter::evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool, int64_t& evictBytes) {
rawPartitionLengths_[partitionId] += blockPayload->rawSize();
ScopedTimer timer(&spillTime_);
ARROW_ASSIGN_OR_RAISE(auto buffer, blockPayload->readBufferAt(0));
bytesEvicted_[partitionId] += rssClient_->pushPartitionData(partitionId, buffer->data_as<char>(), buffer->size());
auto delta = rssClient_->pushPartitionData(partitionId, buffer->data_as<char>(), buffer->size());
bytesEvicted_[partitionId] += delta;
evictBytes += delta;
return arrow::Status::OK();
}

arrow::Status RssPartitionWriter::doEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload) {
arrow::Status RssPartitionWriter::doEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, int64_t& evictBytes) {
rawPartitionLengths_[partitionId] += inMemoryPayload->rawSize();
auto payloadType = codec_ ? Payload::Type::kCompressed : Payload::Type::kUncompressed;
ARROW_ASSIGN_OR_RAISE(
Expand All @@ -117,8 +122,10 @@ arrow::Status RssPartitionWriter::doEvict(uint32_t partitionId, std::unique_ptr<
// Push.
ScopedTimer timer(&spillTime_);
ARROW_ASSIGN_OR_RAISE(auto buffer, rssBufferOs->Finish());
bytesEvicted_[partitionId] += rssClient_->pushPartitionData(
auto delta = rssClient_->pushPartitionData(
partitionId, reinterpret_cast<char*>(const_cast<uint8_t*>(buffer->data())), buffer->size());
bytesEvicted_[partitionId] += delta;
evictBytes += delta;
return arrow::Status::OK();
}
} // namespace gluten
17 changes: 11 additions & 6 deletions cpp/core/shuffle/rss/RssPartitionWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,26 @@ class RssPartitionWriter final : public PartitionWriter {
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
Evict::type evictType,
bool reuseBuffers) override;
bool reuseBuffers,
int64_t& evictBytes) override;

arrow::Status sortEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, bool isFinal)
override;
arrow::Status sortEvict(
uint32_t partitionId,
std::unique_ptr<InMemoryPayload> inMemoryPayload,
bool isFinal,
int64_t& evictBytes) override;

arrow::Status evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop) override;
arrow::Status evict(uint32_t partitionId, std::unique_ptr<BlockPayload> blockPayload, bool stop, int64_t& evictBytes)
override;

arrow::Status reclaimFixedSize(int64_t size, int64_t* actual) override;

arrow::Status stop(ShuffleWriterMetrics* metrics) override;
arrow::Status stop(ShuffleWriterMetrics* metrics, int64_t& evictBytes) override;

private:
void init();

arrow::Status doEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload);
arrow::Status doEvict(uint32_t partitionId, std::unique_ptr<InMemoryPayload> inMemoryPayload, int64_t& evictBytes);

std::shared_ptr<RssPartitionWriterOptions> options_;
std::shared_ptr<RssClient> rssClient_;
Expand Down
2 changes: 1 addition & 1 deletion cpp/velox/benchmarks/GenericBenchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ void populateWriterMetrics(
}
metrics.dataSize +=
std::accumulate(shuffleWriter->rawPartitionLengths().begin(), shuffleWriter->rawPartitionLengths().end(), 0LL);
metrics.bytesWritten += shuffleWriter->totalBytesWritten();
metrics.bytesWritten += shuffleWriter->bytesWritten();
metrics.bytesSpilled += shuffleWriter->totalBytesEvicted();
}

Expand Down
8 changes: 5 additions & 3 deletions cpp/velox/shuffle/VeloxHashShuffleWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ arrow::Result<std::shared_ptr<arrow::Buffer>> VeloxHashShuffleWriter::generateCo
}

arrow::Status VeloxHashShuffleWriter::write(std::shared_ptr<ColumnarBatch> cb, int64_t memLimit) {
writtenBytes_ = 0;
if (partitioning_ == Partitioning::kSingle) {
auto veloxColumnBatch = VeloxColumnarBatch::from(veloxPool_.get(), cb);
VELOX_CHECK_NOT_NULL(veloxColumnBatch);
Expand Down Expand Up @@ -334,6 +335,7 @@ arrow::Status VeloxHashShuffleWriter::partitioningAndDoSplit(facebook::velox::Ro
}

arrow::Status VeloxHashShuffleWriter::stop() {
writtenBytes_ = 0;
setSplitState(SplitState::kStopEvict);
if (partitioning_ != Partitioning::kSingle) {
for (auto pid = 0; pid < numPartitions_; ++pid) {
Expand All @@ -344,7 +346,7 @@ arrow::Status VeloxHashShuffleWriter::stop() {
{
SCOPED_TIMER(cpuWallTimingList_[CpuWallTimingStop]);
setSplitState(SplitState::kStop);
RETURN_NOT_OK(partitionWriter_->stop(&metrics_));
RETURN_NOT_OK(partitionWriter_->stop(&metrics_, writtenBytes_));
partitionBuffers_.clear();
}

Expand Down Expand Up @@ -966,7 +968,7 @@ arrow::Status VeloxHashShuffleWriter::evictBuffers(
if (!buffers.empty()) {
auto payload =
std::make_unique<InMemoryPayload>(numRows, &isValidityBuffer_, schema_, std::move(buffers), hasComplexType_);
RETURN_NOT_OK(partitionWriter_->hashEvict(partitionId, std::move(payload), Evict::kCache, reuseBuffers));
RETURN_NOT_OK(partitionWriter_->hashEvict(partitionId, std::move(payload), Evict::kCache, reuseBuffers, writtenBytes_));
}
return arrow::Status::OK();
}
Expand Down Expand Up @@ -1387,7 +1389,7 @@ arrow::Result<int64_t> VeloxHashShuffleWriter::evictPartitionBuffersMinSize(int6
auto payload = std::make_unique<InMemoryPayload>(
item.second, &isValidityBuffer_, schema_, std::move(buffers), hasComplexType_);
metrics_.totalBytesToEvict += payload->rawSize();
RETURN_NOT_OK(partitionWriter_->hashEvict(pid, std::move(payload), Evict::kSpill, false));
RETURN_NOT_OK(partitionWriter_->hashEvict(pid, std::move(payload), Evict::kSpill, false, writtenBytes_));
evicted = beforeEvict - partitionBufferPool_->bytes_allocated();
if (evicted >= size) {
break;
Expand Down
6 changes: 4 additions & 2 deletions cpp/velox/shuffle/VeloxRssSortShuffleWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ arrow::Status VeloxRssSortShuffleWriter::doSort(facebook::velox::RowVectorPtr rv
}

arrow::Status VeloxRssSortShuffleWriter::write(std::shared_ptr<ColumnarBatch> cb, int64_t /* memLimit */) {
writtenBytes_ = 0;
if (partitioning_ == Partitioning::kSingle) {
auto veloxColumnBatch = VeloxColumnarBatch::from(veloxPool_.get(), cb);
VELOX_CHECK_NOT_NULL(veloxColumnBatch);
Expand Down Expand Up @@ -125,7 +126,7 @@ arrow::Status VeloxRssSortShuffleWriter::evictBatch(uint32_t partitionId) {
auto arrowBuffer = std::make_shared<arrow::Buffer>(buffer->as<uint8_t>(), buffer->size());
ARROW_ASSIGN_OR_RAISE(
auto payload, BlockPayload::fromBuffers(Payload::kRaw, 0, {std::move(arrowBuffer)}, nullptr, nullptr, nullptr));
RETURN_NOT_OK(partitionWriter_->evict(partitionId, std::move(payload), stopped_));
RETURN_NOT_OK(partitionWriter_->evict(partitionId, std::move(payload), stopped_, writtenBytes_));
batch_ = std::make_unique<facebook::velox::VectorStreamGroup>(veloxPool_.get(), serde_.get());
batch_->createStreamTree(rowType_, splitBufferSize_, &serdeOptions_);
return arrow::Status::OK();
Expand Down Expand Up @@ -190,6 +191,7 @@ arrow::Status VeloxRssSortShuffleWriter::evictRowVector(uint32_t partitionId) {
}

arrow::Status VeloxRssSortShuffleWriter::stop() {
writtenBytes_ = 0;
stopped_ = true;
for (auto pid = 0; pid < numPartitions(); ++pid) {
RETURN_NOT_OK(evictRowVector(pid));
Expand All @@ -199,7 +201,7 @@ arrow::Status VeloxRssSortShuffleWriter::stop() {
{
SCOPED_TIMER(cpuWallTimingList_[CpuWallTimingStop]);
setSortState(RssSortState::kSortStop);
RETURN_NOT_OK(partitionWriter_->stop(&metrics_));
RETURN_NOT_OK(partitionWriter_->stop(&metrics_, writtenBytes_));
}

stat();
Expand Down
Loading