Skip to content

Commit

Permalink
Skip next row vector free when clear row container (facebookincubator…
Browse files Browse the repository at this point in the history
…#11101)

Summary: Pull Request resolved: facebookincubator#11101

Reviewed By: Yuhta

Differential Revision: D63442387

fbshipit-source-id: 9166e485ec1605913c3706256a9a9f532d914a45
  • Loading branch information
xiaoxmeng authored and athmaja-n committed Jan 10, 2025
1 parent 81c240c commit 2bcdcaa
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 27 deletions.
36 changes: 11 additions & 25 deletions velox/exec/RowContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ char* RowContainer::initializeRow(char* row, bool reuse) {
}

void RowContainer::eraseRows(folly::Range<char**> rows) {
freeRowsExtraMemory(rows, false);
freeRowsExtraMemory(rows, /*freeNextRowVector=*/true);
for (auto* row : rows) {
VELOX_CHECK(!bits::isBitSet(row, freeFlagOffset_), "Double free of row");
bits::setBit(row, freeFlagOffset_);
Expand Down Expand Up @@ -457,31 +457,11 @@ void RowContainer::freeAggregates(folly::Range<char**> rows) {
}
}

void RowContainer::freeNextRowVectors(folly::Range<char**> rows, bool clear) {
void RowContainer::freeNextRowVectors(folly::Range<char**> rows) {
if (!nextOffset_ || !hasDuplicateRows_) {
return;
}

if (clear) {
for (auto row : rows) {
auto vector = getNextRowVector(row);
if (vector) {
// Clear all rows, we can clear the nextOffset_ slots and delete the
// next-row-vector.
for (auto& next : *vector) {
getNextRowVector(next) = nullptr;
}
// Because of 'parallelJoinBuild', the memory for the next row vector
// may not be allocated from the RowContainer to which the row belongs,
// hence we need to release memory through the vector's allocator.
auto allocator = vector->get_allocator().allocator();
std::destroy_at(vector);
allocator->free(HashStringAllocator::headerOf(vector));
}
}
return;
}

for (auto row : rows) {
auto vector = getNextRowVector(row);
if (vector) {
Expand All @@ -500,10 +480,14 @@ void RowContainer::freeNextRowVectors(folly::Range<char**> rows, bool clear) {
}
}

void RowContainer::freeRowsExtraMemory(folly::Range<char**> rows, bool clear) {
void RowContainer::freeRowsExtraMemory(
folly::Range<char**> rows,
bool freeNextRowVector) {
freeVariableWidthFields(rows);
freeAggregates(rows);
freeNextRowVectors(rows, clear);
if (freeNextRowVector) {
freeNextRowVectors(rows);
}
numRows_ -= rows.size();
}

Expand Down Expand Up @@ -959,7 +943,9 @@ void RowContainer::clear() {
std::vector<char*> rows(kBatch);
RowContainerIterator iter;
while (auto numRows = listRows(&iter, kBatch, rows.data())) {
freeRowsExtraMemory(folly::Range<char**>(rows.data(), numRows), true);
freeRowsExtraMemory(
folly::Range<char**>(rows.data(), numRows),
/*freeNextRowVector=*/false);
}
}
hasDuplicateRows_ = false;
Expand Down
4 changes: 2 additions & 2 deletions velox/exec/RowContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1368,9 +1368,9 @@ class RowContainer {
void freeAggregates(folly::Range<char**> rows);

// Free next row vectors associated with the 'rows'.
void freeNextRowVectors(folly::Range<char**> rows, bool clear);
void freeNextRowVectors(folly::Range<char**> rows);

void freeRowsExtraMemory(folly::Range<char**> rows, bool clear);
void freeRowsExtraMemory(folly::Range<char**> rows, bool freeNextRowVector);

// Updates the specific column's columnHasNulls_ flag, if 'hasNulls' is true.
// columnHasNulls_ flag is false by default.
Expand Down

0 comments on commit 2bcdcaa

Please sign in to comment.