Skip to content

Commit

Permalink
ARROW-85: memcmp can be avoided in Equal when comparing with the same …
Browse files Browse the repository at this point in the history
Avoid memcmp when possible, if the two underlying buffers are the same.

Author: Kai Zheng <kai.zheng@intel.com>

Closes #57 from drankye/upstream and squashes the following commits:

2a70944 [Kai Zheng] Free test buffers afterwards
6a8bef5 [Kai Zheng] Fixed some comments
b83f989 [Kai Zheng] ARROW-85. Corrected another format issue by clang-format
0ddcd01 [Kai Zheng] ARROW-85. Fixed another format issue
1b48663 [Kai Zheng] ARROW-85. Fixed checking styles
9f239a3 [Kai Zheng] ARROW-85. Added tests for Buffer and the new behavior
4d04c27 [Kai Zheng] ARROW-85 memcmp can be avoided in Equal when comparing with the same Buffer
  • Loading branch information
Kai Zheng authored and wesm committed Apr 11, 2016
1 parent 9d88a50 commit 7b2153b
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
43 changes: 43 additions & 0 deletions cpp/src/arrow/util/buffer-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,47 @@ TEST_F(TestBuffer, ResizeOOM) {
ASSERT_RAISES(OutOfMemory, buf.Resize(to_alloc));
}

TEST_F(TestBuffer, EqualsWithSameContent) {
MemoryPool* pool = default_memory_pool();
const int32_t bufferSize = 128 * 1024;
uint8_t* rawBuffer1;
ASSERT_OK(pool->Allocate(bufferSize, &rawBuffer1));
memset(rawBuffer1, 12, bufferSize);
uint8_t* rawBuffer2;
ASSERT_OK(pool->Allocate(bufferSize, &rawBuffer2));
memset(rawBuffer2, 12, bufferSize);
uint8_t* rawBuffer3;
ASSERT_OK(pool->Allocate(bufferSize, &rawBuffer3));
memset(rawBuffer3, 3, bufferSize);

Buffer buffer1(rawBuffer1, bufferSize);
Buffer buffer2(rawBuffer2, bufferSize);
Buffer buffer3(rawBuffer3, bufferSize);
ASSERT_TRUE(buffer1.Equals(buffer2));
ASSERT_FALSE(buffer1.Equals(buffer3));

pool->Free(rawBuffer1, bufferSize);
pool->Free(rawBuffer2, bufferSize);
pool->Free(rawBuffer3, bufferSize);
}

TEST_F(TestBuffer, EqualsWithSameBuffer) {
MemoryPool* pool = default_memory_pool();
const int32_t bufferSize = 128 * 1024;
uint8_t* rawBuffer;
ASSERT_OK(pool->Allocate(bufferSize, &rawBuffer));
memset(rawBuffer, 111, bufferSize);

Buffer buffer1(rawBuffer, bufferSize);
Buffer buffer2(rawBuffer, bufferSize);
ASSERT_TRUE(buffer1.Equals(buffer2));

const int64_t nbytes = bufferSize / 2;
Buffer buffer3(rawBuffer, nbytes);
ASSERT_TRUE(buffer1.Equals(buffer3, nbytes));
ASSERT_FALSE(buffer1.Equals(buffer3, nbytes + 1));

pool->Free(rawBuffer, bufferSize);
}

} // namespace arrow
9 changes: 6 additions & 3 deletions cpp/src/arrow/util/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,15 @@ class Buffer : public std::enable_shared_from_this<Buffer> {
// Return true if both buffers are the same size and contain the same bytes
// up to the number of compared bytes
bool Equals(const Buffer& other, int64_t nbytes) const {
return this == &other || (size_ >= nbytes && other.size_ >= nbytes &&
!memcmp(data_, other.data_, nbytes));
return this == &other ||
(size_ >= nbytes && other.size_ >= nbytes &&
(data_ == other.data_ || !memcmp(data_, other.data_, nbytes)));
}

bool Equals(const Buffer& other) const {
return this == &other || (size_ == other.size_ && !memcmp(data_, other.data_, size_));
return this == &other ||
(size_ == other.size_ &&
(data_ == other.data_ || !memcmp(data_, other.data_, size_)));
}

const uint8_t* data() const { return data_; }
Expand Down

0 comments on commit 7b2153b

Please sign in to comment.