Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-67: C++ metadata flatbuffer serialization and data movement to memory maps #28

Closed
wants to merge 3 commits into from
+3,113 −838
Closed

ARROW-67: C++ metadata flatbuffer serialization and data movement to memory maps #28

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fa6319
ARROW-28: Draft C++ shared memory IPC workflow and related refactoring /
wesm Mar 12, 2016
5e841f7
Create explicit PrimitiveArray subclasses to avoid unwanted template …
wesm Mar 23, 2016
0cef7ea
Add NullArray type now that Array is virtual, fix pyarrow build
wesm Mar 23, 2016
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
4 changes: 1 addition & 3 deletions cpp/src/arrow/array-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@

namespace arrow {

static TypePtr int32 = TypePtr(new Int32Type());

class TestArray : public ::testing::Test {
public:
void SetUp() {
Expand Down Expand Up @@ -76,7 +74,7 @@ TEST_F(TestArray, TestIsNull) {
std::shared_ptr<Buffer> null_buf = test::bytes_to_null_buffer(nulls.data(),
nulls.size());
std::unique_ptr<Array> arr;
arr.reset(new Array(int32, nulls.size(), null_count, null_buf));
arr.reset(new Int32Array(nulls.size(), nullptr, null_count, null_buf));

ASSERT_EQ(null_count, arr->null_count());
ASSERT_EQ(5, null_buf->size());
Expand Down
8 changes: 1 addition & 7 deletions cpp/src/arrow/array.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Array::Array(const TypePtr& type, int32_t length, int32_t null_count,
}
}

bool Array::Equals(const Array& other) const {
bool Array::EqualsExact(const Array& other) const {
if (this == &other) return true;
if (length_ != other.length_ || null_count_ != other.null_count_ ||
type_enum() != other.type_enum()) {
Expand All @@ -50,10 +50,4 @@ bool Array::Equals(const Array& other) const {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any reason you aren't comparing length here as well? I suppose its subsumed by the nulls_ check when there are nulls, but what happens if there aren't? I suppose we would expect classes that inherit from array to check it as well. In either case, it might be worth documenting that this is intentional if it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

An oversight. I will correct this and address the below comments, too

}

bool Array::Equals(const std::shared_ptr<Array>& arr) const {
if (arr.get() == nullptr) return false;
if (this == arr.get()) return true;
return Equals(*static_cast<const Array*>(arr.get()));
}

} // namespace arrow
4 changes: 2 additions & 2 deletions cpp/src/arrow/array.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ class Array {
return nulls_;
}

bool Equals(const Array& arr) const;
virtual bool Equals(const std::shared_ptr<Array>& arr) const;
bool EqualsExact(const Array& arr) const;
virtual bool Equals(const std::shared_ptr<Array>& arr) const = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

Clang revealed that this (plus the downstream PrimitiveArrayImpl templates) was causing some bad behavior, so we can review in the future how to have the cleanest code for the proliferation of array containers while avoiding unwanted template instantiation (I admit I don't know enough about this to make a judgment


protected:
TypePtr type_;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/types/boolean.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

namespace arrow {

typedef PrimitiveArrayImpl<BooleanType> BooleanArray;
// typedef PrimitiveArrayImpl<BooleanType> BooleanArray;

class BooleanBuilder : public ArrayBuilder {
};
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/types/list.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

namespace arrow {

bool ListArray::Equals(const ListArray& other) const {
bool ListArray::EqualsExact(const ListArray& other) const {
if (this == &other) return true;
if (null_count_ != other.null_count_) {
return false;
Expand All @@ -45,7 +45,7 @@ bool ListArray::Equals(const std::shared_ptr<Array>& arr) const {
if (this->type_enum() != arr->type_enum()) {
return false;
}
return Equals(*static_cast<const ListArray*>(arr.get()));
return EqualsExact(*static_cast<const ListArray*>(arr.get()));
}

} // namespace arrow
2 changes: 1 addition & 1 deletion cpp/src/arrow/types/list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ListArray : public Array {
int32_t value_offset(int i) { return offsets_[i];}
int32_t value_length(int i) { return offsets_[i + 1] - offsets_[i];}

bool Equals(const ListArray& other) const;
bool EqualsExact(const ListArray& other) const;
bool Equals(const std::shared_ptr<Array>& arr) const override;

protected:
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/types/primitive-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class TestPrimitiveBuilder : public TestBuilder {
ASSERT_EQ(0, builder_->null_count());
ASSERT_EQ(nullptr, builder_->buffer());

ASSERT_TRUE(result->Equals(*expected.get()));
ASSERT_TRUE(result->EqualsExact(*expected.get()));
ASSERT_EQ(ex_null_count, result->null_count());
}

Expand All @@ -143,7 +143,7 @@ class TestPrimitiveBuilder : public TestBuilder {
ASSERT_EQ(0, builder_nn_->capacity());
ASSERT_EQ(nullptr, builder_nn_->buffer());

ASSERT_TRUE(result->Equals(*expected.get()));
ASSERT_TRUE(result->EqualsExact(*expected.get()));
ASSERT_EQ(0, result->null_count());
}

Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/types/primitive.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ PrimitiveArray::PrimitiveArray(const TypePtr& type, int32_t length,
raw_data_ = data == nullptr? nullptr : data_->data();
}

bool PrimitiveArray::Equals(const PrimitiveArray& other) const {
bool PrimitiveArray::EqualsExact(const PrimitiveArray& other) const {
if (this == &other) return true;
if (null_count_ != other.null_count_) {
return false;
Expand All @@ -55,7 +55,7 @@ bool PrimitiveArray::Equals(const std::shared_ptr<Array>& arr) const {
if (this->type_enum() != arr->type_enum()) {
return false;
}
return Equals(*static_cast<const PrimitiveArray*>(arr.get()));
return EqualsExact(*static_cast<const PrimitiveArray*>(arr.get()));
}

} // namespace arrow
87 changes: 34 additions & 53 deletions cpp/src/arrow/types/primitive.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,51 +45,49 @@ class PrimitiveArray : public Array {

const std::shared_ptr<Buffer>& data() const { return data_;}

bool Equals(const PrimitiveArray& other) const;
bool EqualsExact(const PrimitiveArray& other) const;
bool Equals(const std::shared_ptr<Array>& arr) const override;

protected:
std::shared_ptr<Buffer> data_;
const uint8_t* raw_data_;
};


template <typename TypeClass>
class PrimitiveArrayImpl : public PrimitiveArray {
public:
typedef typename TypeClass::c_type value_type;

PrimitiveArrayImpl(const TypePtr& type, int32_t length,
const std::shared_ptr<Buffer>& data,
int32_t null_count = 0,
const std::shared_ptr<Buffer>& nulls = nullptr) :
PrimitiveArray(type, length, data, null_count, nulls) {}

PrimitiveArrayImpl(int32_t length, const std::shared_ptr<Buffer>& data,
int32_t null_count = 0,
const std::shared_ptr<Buffer>& nulls = nullptr) :
PrimitiveArray(std::make_shared<TypeClass>(), length, data,
null_count, nulls) {}

virtual ~PrimitiveArrayImpl() {}

bool Equals(const PrimitiveArrayImpl& other) const {
return PrimitiveArray::Equals(*static_cast<const PrimitiveArray*>(&other));
}

const value_type* raw_data() const {
return reinterpret_cast<const value_type*>(raw_data_);
}

value_type Value(int i) const {
return raw_data()[i];
}

TypeClass* exact_type() const {
return static_cast<TypeClass*>(type_);
}
#define NUMERIC_ARRAY_DECL(NAME, TypeClass, T) \
class NAME : public PrimitiveArray { \
public: \
using value_type = T; \
using PrimitiveArray::PrimitiveArray; \
NAME(int32_t length, const std::shared_ptr<Buffer>& data, \
int32_t null_count = 0, \
const std::shared_ptr<Buffer>& nulls = nullptr) : \
PrimitiveArray(std::make_shared<TypeClass>(), length, data, \
null_count, nulls) {} \
\
bool EqualsExact(const NAME& other) const { \
return PrimitiveArray::EqualsExact( \
*static_cast<const PrimitiveArray*>(&other)); \
} \
\
const T* raw_data() const { \
return reinterpret_cast<const T*>(raw_data_); \
} \
\
T Value(int i) const { \
return raw_data()[i]; \
} \
};

NUMERIC_ARRAY_DECL(UInt8Array, UInt8Type, uint8_t);
NUMERIC_ARRAY_DECL(Int8Array, Int8Type, int8_t);
NUMERIC_ARRAY_DECL(UInt16Array, UInt16Type, uint16_t);
NUMERIC_ARRAY_DECL(Int16Array, Int16Type, int16_t);
NUMERIC_ARRAY_DECL(UInt32Array, UInt32Type, uint32_t);
NUMERIC_ARRAY_DECL(Int32Array, Int32Type, int32_t);
NUMERIC_ARRAY_DECL(UInt64Array, UInt64Type, uint64_t);
NUMERIC_ARRAY_DECL(Int64Array, Int64Type, int64_t);
NUMERIC_ARRAY_DECL(FloatArray, FloatType, float);
NUMERIC_ARRAY_DECL(DoubleArray, DoubleType, double);

template <typename Type, typename ArrayType>
class PrimitiveBuilder : public ArrayBuilder {
Expand Down Expand Up @@ -217,23 +215,6 @@ class PrimitiveBuilder : public ArrayBuilder {
int elsize_;
};

// Array containers

typedef PrimitiveArrayImpl<UInt8Type> UInt8Array;
typedef PrimitiveArrayImpl<Int8Type> Int8Array;

typedef PrimitiveArrayImpl<UInt16Type> UInt16Array;
typedef PrimitiveArrayImpl<Int16Type> Int16Array;

typedef PrimitiveArrayImpl<UInt32Type> UInt32Array;
typedef PrimitiveArrayImpl<Int32Type> Int32Array;

typedef PrimitiveArrayImpl<UInt64Type> UInt64Array;
typedef PrimitiveArrayImpl<Int64Type> Int64Array;

typedef PrimitiveArrayImpl<FloatType> FloatArray;
typedef PrimitiveArrayImpl<DoubleType> DoubleArray;

// Builders

typedef PrimitiveBuilder<UInt8Type, UInt8Array> UInt8Builder;
Expand Down