Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
marin-ma committed Aug 20, 2024
1 parent 10f92bf commit e80028f
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 33 deletions.
41 changes: 22 additions & 19 deletions velox/row/CompactRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
#include "velox/row/CompactRow.h"
#include "velox/common/base/RawVector.h"
#include "velox/vector/FlatVector.h"

namespace facebook::velox::row {
Expand Down Expand Up @@ -326,25 +327,26 @@ int32_t CompactRow::serializeRow(vector_size_t index, char* buffer) {
}

void CompactRow::serializeRow(
const IndexRange& indexRange,
vector_size_t offset,
vector_size_t size,
char* buffer,
std::vector<size_t>& offsets) {
VELOX_CHECK_EQ(offsets.size(), indexRange.size);
std::vector<size_t>& bufferOffsets) {
VELOX_CHECK_EQ(bufferOffsets.size(), size);

raw_vector<vector_size_t> rows(indexRange.size);
raw_vector<uint8_t*> nulls(indexRange.size);
raw_vector<vector_size_t> rows(size);
raw_vector<uint8_t*> nulls(size);
if (decoded_.isIdentityMapping()) {
std::iota(rows.begin(), rows.end(), indexRange.begin);
std::iota(rows.begin(), rows.end(), offset);
} else {
for (auto i = 0; i < indexRange.size; ++i) {
rows[i] = decoded_.index(indexRange.begin + i);
for (auto i = 0; i < size; ++i) {
rows[i] = decoded_.index(offset + i);
}
}

auto* base = reinterpret_cast<uint8_t*>(buffer);
for (auto i = 0; i < indexRange.size; ++i) {
nulls[i] = base + offsets[i];
offsets[i] += rowNullBytes_;
for (auto i = 0; i < size; ++i) {
nulls[i] = base + bufferOffsets[i];
bufferOffsets[i] += rowNullBytes_;
}

for (auto childIdx = 0; childIdx < children_.size(); ++childIdx) {
Expand All @@ -361,17 +363,17 @@ void CompactRow::serializeRow(
child.valueBytes_,
nulls,
buffer,
offsets);
bufferOffsets);
} else {
auto mayHaveNulls = child.decoded_.mayHaveNulls();
for (auto i = 0; i < indexRange.size; ++i) {
for (auto i = 0; i < size; ++i) {
if (mayHaveNulls && child.isNullAt(rows[i])) {
bits::setBit(nulls[i], childIdx, true);
} else {
// Write non-null variable-width value.
auto size =
child.serializeVariableWidth(rows[i], buffer + offsets[i]);
offsets[i] += size;
auto bytes =
child.serializeVariableWidth(rows[i], buffer + bufferOffsets[i]);
bufferOffsets[i] += bytes;
}
}
}
Expand Down Expand Up @@ -600,10 +602,11 @@ int32_t CompactRow::serialize(vector_size_t index, char* buffer) {
}

void CompactRow::serialize(
const IndexRange& indexRange,
vector_size_t offset,
vector_size_t size,
char* buffer,
std::vector<size_t>& offsets) {
return serializeRow(indexRange, buffer, offsets);
std::vector<size_t>& bufferOffsets) {
return serializeRow(offset, size, buffer, bufferOffsets);
}

void CompactRow::serializeFixedWidth(vector_size_t index, char* buffer) {
Expand Down
22 changes: 12 additions & 10 deletions velox/row/CompactRow.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "velox/vector/ComplexVector.h"
#include "velox/vector/DecodedVector.h"
#include "velox/vector/VectorStream.h"

namespace facebook::velox::row {

Expand All @@ -37,14 +36,15 @@ class CompactRow {
/// 'buffer' must have sufficient capacity and set to all zeros.
int32_t serialize(vector_size_t index, char* buffer);

/// Serializes rows at specified index range into 'buffer' at given offset.
/// 'buffer' must have sufficient capacity and set to all zeros.
/// The size of 'offsets' and indexRange must be the same.
/// The value of 'offsets' will be updated by the actual bytes written.
/// Serializes rows in range [offset, offset + size) into 'buffer' at given
/// 'bufferOffsets'. 'buffer' must have sufficient capacity and set to all
/// zeros. The size of 'bufferOffsets' and 'size' must be the same.
/// 'bufferOffsets' will be updated by the actual bytes written.
void serialize(
const IndexRange& indexRange,
vector_size_t offset,
vector_size_t size,
char* buffer,
std::vector<size_t>& offsets);
std::vector<size_t>& bufferOffsets);

/// Deserializes multiple rows into a RowVector of specified type. The type
/// must match the contents of the serialized rows.
Expand Down Expand Up @@ -118,11 +118,13 @@ class CompactRow {
/// Serializes struct value to buffer. Value must not be null.
int32_t serializeRow(vector_size_t index, char* buffer);

// Serializes struct value to buffer. Value must not be null.
/// Serializes struct values in range [offset, offset + size) to buffer.
/// Value must not be null.
void serializeRow(
const IndexRange& indexRange,
vector_size_t offset,
vector_size_t size,
char* buffer,
std::vector<size_t>& offsets);
std::vector<size_t>& bufferOffsets);

const TypeKind typeKind_;
DecodedVector decoded_;
Expand Down
3 changes: 1 addition & 2 deletions velox/row/benchmark/UnsafeRowSerializeBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,7 @@ class SerializeBenchmark {
BufferPtr& buffer,
std::vector<size_t>& offsets) {
auto rawBuffer = buffer->asMutable<char>();
IndexRange indexRange{0, numRows};
compactRow.serialize(indexRange, rawBuffer, offsets);
compactRow.serialize(0, numRows, rawBuffer, offsets);
VELOX_CHECK_EQ(buffer->size(), offsets.back());

std::vector<std::string_view> serialized;
Expand Down
3 changes: 1 addition & 2 deletions velox/row/tests/CompactRowTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ class CompactRowTest : public ::testing::Test, public VectorTestBase {

auto rowType = asRowType(data->type());
auto numRows = data->size();
IndexRange range{0, numRows};
std::vector<size_t> offsets(numRows);

CompactRow row(data);
Expand Down Expand Up @@ -90,7 +89,7 @@ class CompactRowTest : public ::testing::Test, public VectorTestBase {
memset(rawBuffer, 0, totalSize);
{
std::vector<std::string_view> serialized;
row.serialize(range, rawBuffer, offsets);
row.serialize(0, numRows, rawBuffer, offsets);
serialized.push_back(std::string_view(rawBuffer, offsets[0]));
for (auto i = 1; i < numRows; ++i) {
serialized.push_back(std::string_view(
Expand Down

0 comments on commit e80028f

Please sign in to comment.