Skip to content

Commit

Permalink
Specialize *Protocol::readStringBody for std::string
Browse files Browse the repository at this point in the history
Summary: `folly::resizeWithoutInitialization` allows us to make this slightly more efficient than the general code

Reviewed By: robertroeser

Differential Revision: D68296464

fbshipit-source-id: a33d849336caa497b256bc5dab6e68559776d0d3
  • Loading branch information
iahs authored and facebook-github-bot committed Jan 30, 2025
1 parent 7744129 commit 3f089ed
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -580,32 +580,7 @@ inline void BinaryProtocolReader::readBinary(folly::IOBuf& str) {
template <typename StrType>
inline void BinaryProtocolReader::readStringBody(StrType& str, int32_t size) {
checkStringSize(size);

// Catch empty string case
if (size == 0) {
str.clear();
return;
}

if (static_cast<int32_t>(in_.length()) < size) {
if (!in_.canAdvance(size)) {
protocol::TProtocolException::throwTruncatedData();
}
str.reserve(size); // only reserve for multi iter case below
}
str.clear();
size_t size_left = size;
while (size_left > 0) {
auto data = in_.peekBytes();
auto data_avail = std::min(data.size(), size_left);
if (data.empty()) {
TProtocolException::throwTruncatedData();
}

str.append((const char*)data.data(), data_avail);
size_left -= data_avail;
in_.skipNoAdvance(data_avail);
}
detail::readStringBody(str, in_, size);
}

inline bool BinaryProtocolReader::advanceToNextField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -736,25 +736,8 @@ inline void CompactProtocolReader::readStringSize(int32_t& size) {

template <typename StrType>
inline void CompactProtocolReader::readStringBody(StrType& str, int32_t size) {
if (static_cast<int32_t>(in_.length()) < size) {
if (!in_.canAdvance(size)) {
protocol::TProtocolException::throwTruncatedData();
}
str.reserve(size); // only reserve for multi iter case below
}
str.clear();
size_t size_left = size;
while (size_left > 0) {
auto data = in_.peekBytes();
auto data_avail = std::min(data.size(), size_left);
if (data.empty()) {
TProtocolException::throwTruncatedData();
}

str.append((const char*)data.data(), data_avail);
size_left -= data_avail;
in_.skipNoAdvance(data_avail);
}
assert(size >= 0); // Checked in readStringSize
detail::readStringBody(str, in_, size);
}

template <typename StrType>
Expand Down
45 changes: 43 additions & 2 deletions third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
#include <sys/types.h>

#include <cstdint>
#include <map>
#include <memory>
#include <string>
#include <vector>

#include <glog/logging.h>

#include <folly/io/Cursor.h>
#include <folly/io/IOBuf.h>
#include <folly/io/IOBufQueue.h>
#include <folly/memory/UninitializedMemoryHacks.h>
#include <folly/portability/GFlags.h>
#include <thrift/lib/cpp/Thrift.h>
#include <thrift/lib/cpp/protocol/TProtocol.h>
Expand Down Expand Up @@ -400,6 +400,47 @@ struct StringTraits<std::unique_ptr<folly::IOBuf>> {
}
};

namespace detail {

template <typename StrType>
inline void readStringBody(StrType& str, folly::io::Cursor& in, size_t size) {
// Catch empty string case
if (size == 0) {
str.clear();
return;
}

if (in.length() < size) {
if (!in.canAdvance(size)) {
protocol::TProtocolException::throwTruncatedData();
}
str.reserve(size); // only reserve for multi iter case below
}
str.clear();
size_t size_left = size;
while (size_left > 0) {
auto data = in.peekBytes();
auto data_avail = std::min(data.size(), size_left);
if (data.empty()) {
TProtocolException::throwTruncatedData();
}

str.append((const char*)data.data(), data_avail);
size_left -= data_avail;
in.skipNoAdvance(data_avail);
}
}

inline void readStringBody(
std::string& str, folly::io::Cursor& in, size_t size) {
folly::resizeWithoutInitialization(str, size);
if (in.pullAtMost(str.data(), size) < size) {
str.clear();
protocol::TProtocolException::throwTruncatedData();
}
}
} // namespace detail

} // namespace apache::thrift

#endif

0 comments on commit 3f089ed

Please sign in to comment.