From 6a6efad5faf702f61820268cca8726a83bc6a9d2 Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Wed, 25 May 2016 12:07:07 -0700 Subject: [PATCH] add Cursor::peekBytes() Summary: The existing Cursor::peek() method pre-dates the existence of folly::ByteRange, and so it returns a std::pair containing a pointer and length. This is usually more awkward to use than ByteRange. This adds a peekBytes() method that returns a ByteRange object, and updates all users of peek() in folly to use peekBytes() instead. Eventually I think we should add a FOLLY_DEPRECATED attribute to peek(). I will wait to add this tag until after converting a few other projects to use peekBytes(), though. Reviewed By: alandau Differential Revision: D3337302 fbshipit-source-id: 14220a059d915bf5adc66b8b283f7228b796a4dc --- folly/Subprocess.cpp | 14 +++++----- folly/experimental/bser/Load.cpp | 6 ++--- folly/io/Compression.cpp | 29 +++++++++++---------- folly/io/Cursor.h | 31 +++++++++++++++------- folly/io/IOBuf.cpp | 20 +++++++------- folly/io/test/IOBufCursorBenchmark.cpp | 2 +- folly/io/test/IOBufCursorTest.cpp | 36 ++++++++++++-------------- 7 files changed, 74 insertions(+), 64 deletions(-) diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp index 769cbebb..8190c470 100644 --- a/folly/Subprocess.cpp +++ b/folly/Subprocess.cpp @@ -606,21 +606,23 @@ pid_t Subprocess::pid() const { namespace { -std::pair queueFront(const IOBufQueue& queue) { +ByteRange queueFront(const IOBufQueue& queue) { auto* p = queue.front(); - if (!p) return std::make_pair(nullptr, 0); - return io::Cursor(p).peek(); + if (!p) { + return ByteRange{}; + } + return io::Cursor(p).peekBytes(); } // fd write bool handleWrite(int fd, IOBufQueue& queue) { for (;;) { - auto p = queueFront(queue); - if (p.second == 0) { + auto b = queueFront(queue); + if (b.empty()) { return true; // EOF } - ssize_t n = writeNoInt(fd, p.first, p.second); + ssize_t n = writeNoInt(fd, b.data(), b.size()); if (n == -1 && errno == EAGAIN) { return false; } diff --git a/folly/experimental/bser/Load.cpp b/folly/experimental/bser/Load.cpp index 4d311f9f..2bd473b3 100644 --- a/folly/experimental/bser/Load.cpp +++ b/folly/experimental/bser/Load.cpp @@ -116,8 +116,8 @@ static dynamic decodeTemplate(Cursor& curs) { dynamic obj = dynamic::object; for (auto& name : names) { - auto pair = curs.peek(); - if ((BserType)pair.first[0] == BserType::Skip) { + auto bytes = curs.peekBytes(); + if ((BserType)bytes.at(0) == BserType::Skip) { obj[name.getString()] = nullptr; curs.skipAtMost(1); continue; @@ -176,7 +176,7 @@ static size_t decodeHeader(Cursor& curs) { throw std::runtime_error("invalid BSER magic header"); } - auto enc = (BserType)curs.peek().first[0]; + auto enc = (BserType)curs.peekBytes().at(0); size_t int_size; switch (enc) { case BserType::Int8: diff --git a/folly/io/Compression.cpp b/folly/io/Compression.cpp index 21190f5c..356699cf 100644 --- a/folly/io/Compression.cpp +++ b/folly/io/Compression.cpp @@ -303,12 +303,13 @@ std::unique_ptr LZ4Codec::doUncompress( } } - auto p = cursor.peek(); + auto sp = StringPiece{cursor.peekBytes()}; auto out = IOBuf::create(actualUncompressedLength); - int n = LZ4_decompress_safe(reinterpret_cast(p.first), - reinterpret_cast(out->writableTail()), - p.second, - actualUncompressedLength); + int n = LZ4_decompress_safe( + sp.data(), + reinterpret_cast(out->writableTail()), + sp.size(), + actualUncompressedLength); if (n < 0 || uint64_t(n) != actualUncompressedLength) { throw std::runtime_error(to( @@ -350,9 +351,9 @@ size_t IOBufSnappySource::Available() const { } const char* IOBufSnappySource::Peek(size_t* len) { - auto p = cursor_.peek(); - *len = p.second; - return reinterpret_cast(p.first); + auto sp = StringPiece{cursor_.peekBytes()}; + *len = sp.size(); + return sp.data(); } void IOBufSnappySource::Skip(size_t n) { @@ -908,10 +909,10 @@ std::unique_ptr LZMA2Codec::doUncompress(const IOBuf* data, defaultBufferLength)); bool streamEnd = false; - auto buf = cursor.peek(); - while (buf.second != 0) { - stream.next_in = const_cast(buf.first); - stream.avail_in = buf.second; + auto buf = cursor.peekBytes(); + while (!buf.empty()) { + stream.next_in = const_cast(buf.data()); + stream.avail_in = buf.size(); while (stream.avail_in != 0) { if (streamEnd) { @@ -922,8 +923,8 @@ std::unique_ptr LZMA2Codec::doUncompress(const IOBuf* data, streamEnd = doInflate(&stream, out.get(), defaultBufferLength); } - cursor.skip(buf.second); - buf = cursor.peek(); + cursor.skip(buf.size()); + buf = cursor.peekBytes(); } while (!streamEnd) { diff --git a/folly/io/Cursor.h b/folly/io/Cursor.h index a1309239..5e1ce527 100644 --- a/folly/io/Cursor.h +++ b/folly/io/Cursor.h @@ -86,10 +86,10 @@ class CursorBase { /** * Return the remaining space available in the current IOBuf. * - * May return 0 if the cursor is at the end of an IOBuf. Use peek() instead - * if you want to avoid this. peek() will advance to the next non-empty - * IOBuf (up to the end of the chain) if the cursor is currently pointing at - * the end of a buffer. + * May return 0 if the cursor is at the end of an IOBuf. Use peekBytes() + * instead if you want to avoid this. peekBytes() will advance to the next + * non-empty IOBuf (up to the end of the chain) if the cursor is currently + * pointing at the end of a buffer. */ size_t length() const { return crtBuf_->length() - offset_; @@ -300,15 +300,26 @@ class CursorBase { /** * Return the available data in the current buffer. * If you want to gather more data from the chain into a contiguous region - * (for hopefully zero-copy access), use gather() before peek(). + * (for hopefully zero-copy access), use gather() before peekBytes(). */ - std::pair peek() { + ByteRange peekBytes() { // Ensure that we're pointing to valid data size_t available = length(); while (UNLIKELY(available == 0 && tryAdvanceBuffer())) { available = length(); } - return std::make_pair(data(), available); + return ByteRange{data(), available}; + } + + /** + * Alternate version of peekBytes() that returns a std::pair + * instead of a ByteRange. (This method pre-dates ByteRange.) + * + * This function will eventually be deprecated. + */ + std::pair peek() { + auto bytes = peekBytes(); + return std::make_pair(bytes.data(), bytes.size()); } void clone(std::unique_ptr& buf, size_t len) { @@ -583,9 +594,9 @@ class Writable { size_t pushAtMost(Cursor cursor, size_t len) { size_t written = 0; for(;;) { - auto currentBuffer = cursor.peek(); - const uint8_t* crtData = currentBuffer.first; - size_t available = currentBuffer.second; + auto currentBuffer = cursor.peekBytes(); + const uint8_t* crtData = currentBuffer.data(); + size_t available = currentBuffer.size(); if (available == 0) { // end of buffer chain return written; diff --git a/folly/io/IOBuf.cpp b/folly/io/IOBuf.cpp index 603e75f7..763522b7 100644 --- a/folly/io/IOBuf.cpp +++ b/folly/io/IOBuf.cpp @@ -962,12 +962,12 @@ size_t IOBufHash::operator()(const IOBuf& buf) const { hasher.Init(0, 0); io::Cursor cursor(&buf); for (;;) { - auto p = cursor.peek(); - if (p.second == 0) { + auto b = cursor.peekBytes(); + if (b.empty()) { break; } - hasher.Update(p.first, p.second); - cursor.skip(p.second); + hasher.Update(b.data(), b.size()); + cursor.skip(b.size()); } uint64_t h1; uint64_t h2; @@ -979,16 +979,16 @@ bool IOBufEqual::operator()(const IOBuf& a, const IOBuf& b) const { io::Cursor ca(&a); io::Cursor cb(&b); for (;;) { - auto pa = ca.peek(); - auto pb = cb.peek(); - if (pa.second == 0 && pb.second == 0) { + auto ba = ca.peekBytes(); + auto bb = cb.peekBytes(); + if (ba.empty() && bb.empty()) { return true; - } else if (pa.second == 0 || pb.second == 0) { + } else if (ba.empty() || bb.empty()) { return false; } - size_t n = std::min(pa.second, pb.second); + size_t n = std::min(ba.size(), bb.size()); DCHECK_GT(n, 0); - if (memcmp(pa.first, pb.first, n)) { + if (memcmp(ba.data(), bb.data(), n)) { return false; } ca.skip(n); diff --git a/folly/io/test/IOBufCursorBenchmark.cpp b/folly/io/test/IOBufCursorBenchmark.cpp index 233f800b..559f7a3e 100644 --- a/folly/io/test/IOBufCursorBenchmark.cpp +++ b/folly/io/test/IOBufCursorBenchmark.cpp @@ -70,7 +70,7 @@ BENCHMARK(skipBenchmark, iters) { while (iters--) { Cursor c(iobuf_read_benchmark.get()); for (int i = 0; i < benchmark_size; i++) { - c.peek(); + c.peekBytes(); c.skip(1); } } diff --git a/folly/io/test/IOBufCursorTest.cpp b/folly/io/test/IOBufCursorTest.cpp index ab8d1f83..958f6d45 100644 --- a/folly/io/test/IOBufCursorTest.cpp +++ b/folly/io/test/IOBufCursorTest.cpp @@ -182,10 +182,10 @@ void append(Appender& appender, StringPiece data) { std::string toString(const IOBuf& buf) { std::string str; Cursor cursor(&buf); - std::pair p; - while ((p = cursor.peek()).second) { - str.append(reinterpret_cast(p.first), p.second); - cursor.skip(p.second); + ByteRange b; + while (!(b = cursor.peekBytes()).empty()) { + str.append(reinterpret_cast(b.data()), b.size()); + cursor.skip(b.size()); } return str; } @@ -218,18 +218,15 @@ TEST(IOBuf, PullAndPeek) { { RWPrivateCursor cursor(iobuf1.get()); - auto p = cursor.peek(); - EXPECT_EQ("he", std::string(reinterpret_cast(p.first), - p.second)); - cursor.skip(p.second); - p = cursor.peek(); - EXPECT_EQ("llo ", std::string(reinterpret_cast(p.first), - p.second)); - cursor.skip(p.second); - p = cursor.peek(); - EXPECT_EQ("world", std::string(reinterpret_cast(p.first), - p.second)); - cursor.skip(p.second); + auto b = cursor.peekBytes(); + EXPECT_EQ("he", StringPiece(b)); + cursor.skip(b.size()); + b = cursor.peekBytes(); + EXPECT_EQ("llo ", StringPiece(b)); + cursor.skip(b.size()); + b = cursor.peekBytes(); + EXPECT_EQ("world", StringPiece(b)); + cursor.skip(b.size()); EXPECT_EQ(3, iobuf1->countChainElements()); EXPECT_EQ(11, iobuf1->computeChainDataLength()); } @@ -237,9 +234,8 @@ TEST(IOBuf, PullAndPeek) { { RWPrivateCursor cursor(iobuf1.get()); cursor.gather(11); - auto p = cursor.peek(); - EXPECT_EQ("hello world", std::string(reinterpret_cast(p.first), p.second)); + auto b = cursor.peekBytes(); + EXPECT_EQ("hello world", StringPiece(b)); EXPECT_EQ(1, iobuf1->countChainElements()); EXPECT_EQ(11, iobuf1->computeChainDataLength()); } @@ -377,7 +373,7 @@ TEST(IOBuf, cloneAndInsert) { EXPECT_EQ(7, iobuf1->countChainElements()); EXPECT_EQ(14, iobuf1->computeChainDataLength()); // Check that nextBuf got set correctly to the buffer with 1 byte left - EXPECT_EQ(1, cursor.peek().second); + EXPECT_EQ(1, cursor.peekBytes().size()); cursor.read(); } -- 2.34.1