Hi all,
To integrate Thrift into the XORP router project, the approach I've
taken is to modify XORP's existing IDL generator to produce code which
uses the Thrift protocols and transports directly.
This is mainly because the code base there already does asynchronous I/O
using its own mechanisms, and using Thrift's own generator doesn't
support this use. I explored both of the ASIO drops (from Rush Manbert
and Spotify), and it was pretty clear that moving to ASIO in our tree
would require significantly more refactoring.
The modified XIF parser generates RPC stubs, which call TProtocol
methods, and present exactly the same method signatures as the existing
code, to limit code modification of XORP to only those parts of the tree
needed to implement Thrift as the RPC layer.
As XIF is mostly a subset of Thrift, I wrote a translator which parses
XIF and produces the equivalent .thrift file, which should allow Thrift
clients to talk to these services directly (providing they use
TBinaryProtocol, TFramedTransport, TCP sockets, and know the endpoint
address for each service).
Because of the nature of the service (IP routing and link management),
we get into situations where we need to pass binary blobs back and forth
as part of the RPC calls we're servicing. The XIF IDL already deals with
this situation, however, in XIF the base type used is vector<uint8_t>.
My stub translator compiles just fine, however, I can end up with
constructs like this:
[given 's' as a vector<uint8_t> argument]
nout += outp->writeBinary(string(reinterpret_cast<const
char*>(&s[0]), s.size()));
Now this is pretty nasty, because the C++ standard libary has no 'const
string&' specialization; indeed, 'const string&' just specifies the
storage class -- as far as C++ is concerned, it isn't a distinct type,
and the code construct above will go off to allocate an intermediate
buffer from the C++ runtime heap. Probably not what we want on a
critical path.
I looked around to see if there were a quick fix in terms of a container
adaptor, no dice. This seems to be a more general problem -- when
working with string literals in C++, 'const char*' and a 'size_t' seems
to be about as good as it gets.
It's likely I'd also need to take a similar approach for the read path,
my stub generator currently has to bounce buffer.
The TProtocol::readBinary() method probably needs a little bit more
thinking about, and I'll come back to that -- I notice
TBinaryProtocol::readBinary() is taking pains not to abuse 'auto'
(stack) class storage.
Example:
[given 's' as a vector<uint8_t> argument]
string tmp_s;
nin += inp->readBinary(tmp_s);
s.resize(tmp_s.size());
memcpy(&s[0], tmp_s.data(), tmp_s.size());
Here is a patch (against SVN trunk) for the C++ client library to
support C-style buffers, as well as 'const std::string&'. by overloading
the writeBinary() method in TProtocol and its derived classes.
This seems a more reasonable approach than limiting it to the use of
std::vector<T>; type safety is side-stepped for a very specific reason.
A 'gmake' runs to completion with this patch on FreeBSD 7.2-STABLE/amd64.
Comments, questions, suggestions highly appreciated.
cheers,
BMS
Index: lib/cpp/src/protocol/TBinaryProtocol.h
===================================================================
--- lib/cpp/src/protocol/TBinaryProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TBinaryProtocol.h (working copy)
@@ -135,6 +135,8 @@
uint32_t writeBinary(const std::string& str);
+ uint32_t writeBinary(const uint8_t* data, size_t len);
+
/**
* Reading functions
*/
Index: lib/cpp/src/protocol/TCompactProtocol.h
===================================================================
--- lib/cpp/src/protocol/TCompactProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TCompactProtocol.h (working copy)
@@ -155,6 +155,8 @@
uint32_t writeBinary(const std::string& str);
+ uint32_t writeBinary(const uint8_t* data, size_t len);
+
/**
* These methods are called by structs, but don't actually have any wired
* output or purpose
Index: lib/cpp/src/protocol/TDebugProtocol.h
===================================================================
--- lib/cpp/src/protocol/TDebugProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TDebugProtocol.h (working copy)
@@ -129,6 +129,7 @@
uint32_t writeBinary(const std::string& str);
+ uint32_t writeBinary(const uint8_t* data, size_t len);
private:
void indentUp();
Index: lib/cpp/src/protocol/TDenseProtocol.cpp
===================================================================
--- lib/cpp/src/protocol/TDenseProtocol.cpp (revision 883543)
+++ lib/cpp/src/protocol/TDenseProtocol.cpp (working copy)
@@ -447,13 +447,17 @@
}
uint32_t TDenseProtocol::writeString(const std::string& str) {
+ return TDenseProtocol::writeBinary(str);
+}
+
+uint32_t TDenseProtocol::writeBinary(const uint8_t* data, size_t len) {
checkTType(T_STRING);
stateTransition();
- return subWriteString(str);
+ return TDenseProtocol::subWriteBinary(data, len);
}
uint32_t TDenseProtocol::writeBinary(const std::string& str) {
- return TDenseProtocol::writeString(str);
+ return TDenseProtocol::writeBinary((const uint8_t*)str.data(), str.size());
}
inline uint32_t TDenseProtocol::subWriteI32(const int32_t i32) {
@@ -461,12 +465,15 @@
}
uint32_t TDenseProtocol::subWriteString(const std::string& str) {
- uint32_t size = str.size();
- uint32_t xfer = subWriteI32((int32_t)size);
- if (size > 0) {
- trans_->write((uint8_t*)str.data(), size);
+ return TDenseProtocol::subWriteBinary((const uint8_t*)str.data(),
str.size());
+}
+
+uint32_t TDenseProtocol::subWriteBinary(const uint8_t* data, size_t len) {
+ uint32_t xfer = subWriteI32((int32_t)len);
+ if (len > 0) {
+ trans_->write(data, len);
}
- return xfer + size;
+ return xfer + len;
}
Index: lib/cpp/src/protocol/TDenseProtocol.h
===================================================================
--- lib/cpp/src/protocol/TDenseProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TDenseProtocol.h (working copy)
@@ -140,6 +140,7 @@
virtual uint32_t writeBinary(const std::string& str);
+ virtual uint32_t writeBinary(const uint8_t* data, size_t len);
/*
* Helper writing functions (don't do state transitions).
@@ -148,6 +149,8 @@
inline uint32_t subWriteString(const std::string& str);
+ inline uint32_t subWriteBinary(const uint8_t* data, size_t len);
+
uint32_t subWriteBool(const bool value) {
return TBinaryProtocol::writeBool(value);
}
Index: lib/cpp/src/protocol/TOneWayProtocol.h
===================================================================
--- lib/cpp/src/protocol/TOneWayProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TOneWayProtocol.h (working copy)
@@ -295,6 +295,11 @@
subclass_ + " does not support writing (yet).");
}
+ uint32_t writeBinary(const uint8_t* data, size_t len) {
+ throw TProtocolException(TProtocolException::NOT_IMPLEMENTED,
+ subclass_ + " does not support writing (yet).");
+ }
+
private:
std::string subclass_;
};
Index: lib/cpp/src/protocol/TJSONProtocol.cpp
===================================================================
--- lib/cpp/src/protocol/TJSONProtocol.cpp (revision 883543)
+++ lib/cpp/src/protocol/TJSONProtocol.cpp (working copy)
@@ -431,15 +431,14 @@
return result;
}
-// Write out the contents of the string as JSON string, base64-encoding
+// Write out the contents of the buffer as JSON string, base64-encoding
// the string's contents, and escaping as appropriate
-uint32_t TJSONProtocol::writeJSONBase64(const std::string &str) {
+uint32_t TJSONProtocol::writeJSONBase64(const uint8_t* data, size_t len) {
uint32_t result = context_->write(*trans_);
result += 2; // For quotes
trans_->write(&kJSONStringDelimiter, 1);
uint8_t b[4];
- const uint8_t *bytes = (const uint8_t *)str.c_str();
- uint32_t len = str.length();
+ const uint8_t *bytes = data;
while (len >= 3) {
// Encode 3 bytes at a time
base64_encode(bytes, 3, b);
@@ -654,9 +653,13 @@
}
uint32_t TJSONProtocol::writeBinary(const std::string& str) {
- return writeJSONBase64(str);
+ return writeJSONBase64((const uint8_t *)str.c_str(), str.length());
}
+uint32_t TJSONProtocol::writeBinary(const uint8_t* data, size_t len) {
+ return writeJSONBase64(data, len);
+}
+
/**
* Reading functions
*/
Index: lib/cpp/src/protocol/TJSONProtocol.h
===================================================================
--- lib/cpp/src/protocol/TJSONProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TJSONProtocol.h (working copy)
@@ -111,7 +111,7 @@
uint32_t writeJSONString(const std::string &str);
- uint32_t writeJSONBase64(const std::string &str);
+ uint32_t writeJSONBase64(const uint8_t* data, size_t len);
template <typename NumberType>
uint32_t writeJSONInteger(NumberType num);
@@ -205,6 +205,8 @@
uint32_t writeBinary(const std::string& str);
+ uint32_t writeBinary(const uint8_t* data, size_t len);
+
/**
* Reading functions
*/
Index: lib/cpp/src/protocol/TBinaryProtocol.cpp
===================================================================
--- lib/cpp/src/protocol/TBinaryProtocol.cpp (revision 883543)
+++ lib/cpp/src/protocol/TBinaryProtocol.cpp (working copy)
@@ -151,20 +151,23 @@
return 8;
}
-
uint32_t TBinaryProtocol::writeString(const string& str) {
- uint32_t size = str.size();
- uint32_t result = writeI32((int32_t)size);
- if (size > 0) {
- trans_->write((uint8_t*)str.data(), size);
- }
- return result + size;
+ return TBinaryProtocol::writeBinary((const uint8_t *)str.data(), str.size());
}
uint32_t TBinaryProtocol::writeBinary(const string& str) {
- return TBinaryProtocol::writeString(str);
+ return TBinaryProtocol::writeBinary((const uint8_t *)str.data(), str.size());
}
+uint32_t TBinaryProtocol::writeBinary(const uint8_t* data, size_t len) {
+ uint32_t result = TBinaryProtocol::writeI32((int32_t)len);
+ if (len > 0) {
+ trans_->write(data, len);
+ }
+ return result + len;
+}
+
+
/**
* Reading functions
*/
Index: lib/cpp/src/protocol/TProtocol.h
===================================================================
--- lib/cpp/src/protocol/TProtocol.h (revision 883543)
+++ lib/cpp/src/protocol/TProtocol.h (working copy)
@@ -238,6 +238,8 @@
virtual uint32_t writeBinary(const std::string& str) = 0;
+ virtual uint32_t writeBinary(const uint8_t* data, size_t len) = 0;
+
/**
* Reading functions
*/
Index: lib/cpp/src/protocol/TCompactProtocol.cpp
===================================================================
--- lib/cpp/src/protocol/TCompactProtocol.cpp (revision 883543)
+++ lib/cpp/src/protocol/TCompactProtocol.cpp (working copy)
@@ -223,13 +223,16 @@
* Write a string to the wire with a varint size preceeding.
*/
uint32_t TCompactProtocol::writeString(const std::string& str) {
- return writeBinary(str);
+ return TCompactProtocol::writeBinary((const uint8_t*)str.data(), str.size());
}
uint32_t TCompactProtocol::writeBinary(const std::string& str) {
- uint32_t ssize = str.size();
- uint32_t wsize = writeVarint32(ssize) + ssize;
- trans_->write((uint8_t*)str.data(), ssize);
+ return TCompactProtocol::writeBinary((const uint8_t*)str.data(), str.size());
+}
+
+uint32_t TCompactProtocol::writeBinary(const uint8_t* data, size_t len) {
+ uint32_t wsize = writeVarint32(len) + len;
+ trans_->write(data, len);
return wsize;
}
Index: lib/cpp/src/protocol/TDebugProtocol.cpp
===================================================================
--- lib/cpp/src/protocol/TDebugProtocol.cpp (revision 883543)
+++ lib/cpp/src/protocol/TDebugProtocol.cpp (working copy)
@@ -343,4 +343,10 @@
return TDebugProtocol::writeString(str);
}
+uint32_t TDebugProtocol::writeBinary(const uint8_t* data, size_t len) {
+ // XXX Wastes stack.
+ string tmp((const char *)data, len);
+ return writeBinary(tmp);
+}
+
}}} // apache::thrift::protocol
