BIND 10 trac499, updated. 117fa08fc7f8b7462190b75f99d7604c49f3b7c6 [trac499] Miscellaneous enhancements

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Mar 7 14:46:12 UTC 2011


The branch, trac499 has been updated
       via  117fa08fc7f8b7462190b75f99d7604c49f3b7c6 (commit)
      from  0b42ead35810e82a97561d71309283950ca9d5a3 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 117fa08fc7f8b7462190b75f99d7604c49f3b7c6
Author: Stephen Morris <stephen at isc.org>
Date:   Mon Mar 7 14:45:05 2011 +0000

    [trac499] Miscellaneous enhancements
    
    * Increasing staging buffer size for better performance.
    * Removed some overhead when creating the IOFetchData structure.
    * Extend unit tests.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/asiolink/io_fetch.cc                |   21 +++---
 src/lib/asiolink/io_fetch.h                 |    2 +-
 src/lib/asiolink/tests/io_fetch_unittest.cc |  109 +++++++++++++++++++++------
 3 files changed, 98 insertions(+), 34 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/asiolink/io_fetch.cc b/src/lib/asiolink/io_fetch.cc
index 8b7efab..bc85892 100644
--- a/src/lib/asiolink/io_fetch.cc
+++ b/src/lib/asiolink/io_fetch.cc
@@ -19,8 +19,7 @@
 #include <netinet/in.h>
 
 #include <boost/bind.hpp>
-#include <boost/shared_array.hpp>
-#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 
 #include <dns/message.h>
@@ -67,13 +66,12 @@ struct IOFetchData {
     // actually instantiated depends on whether the fetch is over UDP or TCP,
     // which is not known until construction of the IOFetch.  Use of a shared
     // pointer here is merely to ensure deletion when the data object is deleted.
-    boost::shared_ptr<IOAsioSocket<IOFetch> > socket;
+    boost::scoped_ptr<IOAsioSocket<IOFetch> > socket;
                                             ///< Socket to use for I/O
-    boost::shared_ptr<IOEndpoint> remote;   ///< Where the fetch was sent
+    boost::scoped_ptr<IOEndpoint> remote;   ///< Where the fetch was sent
     isc::dns::Question          question;   ///< Question to be asked
     isc::dns::OutputBufferPtr   msgbuf;     ///< Wire buffer for question
     isc::dns::OutputBufferPtr   received;   ///< Received data put here
-    boost::shared_array<char>   staging;    ///< Temporary array for received data
     IOFetch::Callback*          callback;   ///< Called on I/O Completion
     asio::deadline_timer        timer;      ///< Timer to measure timeouts
     IOFetch::Protocol           protocol;   ///< Protocol being used
@@ -89,6 +87,8 @@ struct IOFetchData {
     // This means that we must make sure that all possible "origins" take the
     // same arguments in their message in the same order.
     isc::log::MessageID         origin;     ///< Origin of last asynchronous I/O
+    uint8_t                     staging[IOFetch::STAGING_LENGTH];
+                                            ///< Temporary array for received data
 
     /// \brief Constructor
     ///
@@ -126,7 +126,7 @@ struct IOFetchData {
         question(query),
         msgbuf(new isc::dns::OutputBuffer(512)),
         received(buff),
-        staging(new char[IOFetch::MIN_LENGTH]),
+
         callback(cb),
         timer(service.get_io_service()),
         protocol(proto),
@@ -135,7 +135,8 @@ struct IOFetchData {
         offset(0),
         stopped(false),
         timeout(wait),
-        origin(ASIO_UNKORIGIN)
+        origin(ASIO_UNKORIGIN),
+        staging()
     {}
 };
 
@@ -235,11 +236,11 @@ IOFetch::operator()(asio::error_code ec, size_t length) {
         data_->cumulative = 0;          // No data yet received
         data_->offset = 0;              // First data into start of buffer
         do {
-            CORO_YIELD data_->socket->asyncReceive(data_->staging.get(),
-                                                   static_cast<size_t>(MIN_LENGTH),
+            CORO_YIELD data_->socket->asyncReceive(data_->staging,
+                                                   static_cast<size_t>(STAGING_LENGTH),
                                                    data_->offset,
                                                    data_->remote.get(), *this);
-        } while (!data_->socket->processReceivedData(data_->staging.get(), length,
+        } while (!data_->socket->processReceivedData(data_->staging, length,
                                                      data_->cumulative, data_->offset,
                                                      data_->expected, data_->received));
 
diff --git a/src/lib/asiolink/io_fetch.h b/src/lib/asiolink/io_fetch.h
index ab7e9ad..0723777 100644
--- a/src/lib/asiolink/io_fetch.h
+++ b/src/lib/asiolink/io_fetch.h
@@ -78,7 +78,7 @@ public:
 
     /// \brief Integer Constants
     enum {
-        MIN_LENGTH = 4096   ///< Minimum size of receive buffer
+        STAGING_LENGTH = 8192   ///< Size of staging buffer
     };
 
     /// \brief I/O Fetch Callback
diff --git a/src/lib/asiolink/tests/io_fetch_unittest.cc b/src/lib/asiolink/tests/io_fetch_unittest.cc
index df94217..4282c80 100644
--- a/src/lib/asiolink/tests/io_fetch_unittest.cc
+++ b/src/lib/asiolink/tests/io_fetch_unittest.cc
@@ -49,9 +49,8 @@ namespace asiolink {
 
 const asio::ip::address TEST_HOST(asio::ip::address::from_string("127.0.0.1"));
 const uint16_t TEST_PORT(5301);
-// FIXME Shouldn't we send something that is real message?
-const char TEST_DATA[] = "Test response from server to client (longer than 30 bytes)";
-const int SEND_INTERVAL = 250;   // Interval in ms between TCP sends
+const int SEND_INTERVAL = 250;      // Interval in ms between TCP sends
+const size_t MAX_SIZE = 64 * 1024;  // Should be able to take 64kB
 
 // The tests are complex, so debug output has been left in (although disabled).
 // Set this to true to enable it.
@@ -76,12 +75,13 @@ public:
     // The next member is the buffer in which the "server" (implemented by the
     // response handler methods in this class) receives the question sent by the
     // fetch object.
-    uint8_t         receive_buffer_[512];   ///< Server receive buffer
+    uint8_t         receive_buffer_[MAX_SIZE]; ///< Server receive buffer
     vector<uint8_t> send_buffer_;           ///< Server send buffer
     uint16_t        send_cumulative_;       ///< Data sent so far
 
     // Other data.
     string          return_data_;           ///< Data returned by server
+    string          test_data_;             ///< Large string - here for convenience
     bool            debug_;                 ///< true to enable debug output
 
     /// \brief Constructor
@@ -103,7 +103,8 @@ public:
         receive_buffer_(),
         send_buffer_(),
         send_cumulative_(0),
-        return_data_(TEST_DATA),
+        return_data_(""),
+        test_data_(""),
         debug_(DEBUG)
     {
         // Construct the data buffer for question we expect to receive.
@@ -115,6 +116,20 @@ public:
         msg.addQuestion(question_);
         MessageRenderer renderer(*msgbuf_);
         msg.toWire(renderer);
+
+        // Initialize the test data to be returned: tests will return a
+        // substring of this data. (It's convenient to have this as a member of
+        // the class.)
+        //
+        // We could initialize the data with a single character, but as an added
+        // check we'll make ssre that it has some structure.
+
+        test_data_.clear();
+        test_data_.reserve(MAX_SIZE);
+        while (test_data_.size() < MAX_SIZE) {
+            test_data_ += "A message to be returned to the client that has "
+                          "some sort of structure.";
+        }
     }
 
     /// \brief UDP Response handler (the "remote UDP DNS server")
@@ -237,7 +252,8 @@ public:
     ///
     /// Send the TCP data back to the IOFetch object.  The data is sent in
     /// three chunks - two of 16 bytes and the remainder, with a 250ms gap
-    /// between each.
+    /// between each. (Amounts of data smaller than one 32 bytes are sent in
+    /// one or two packets.)
     ///
     /// \param socket Socket over which send should take place
     void tcpSendData(tcp::socket* socket) {
@@ -245,11 +261,20 @@ public:
             cout << "tcpSendData()" << endl;
         }
 
-        // Decide what to send based on the cumulative count
+        // Decide what to send based on the cumulative count.  At most we'll do
+        // two chunks of 16 bytes (with a 250ms gap between) and then the
+        // remainder.
         uint8_t* send_ptr = &send_buffer_[send_cumulative_];
                                     // Pointer to data to send
         size_t amount = 16;         // Amount of data to send
-        if (send_cumulative_ > 30) {
+        if (send_cumulative_ < (2 * amount)) {
+            
+            // First or second time through, send at most 16 bytes
+            amount = min(amount, (send_buffer_.size() - send_cumulative_));
+
+        } else {
+
+            // Third time through, send the remainder.
             amount = send_buffer_.size() - send_cumulative_;
         }
 
@@ -403,6 +428,9 @@ public:
     ///
     /// \param Test data to return to client
     void tcpSendReturnTest(const std::string& return_data) {
+        if (debug_) {
+            cout << "tcpSendReturnTest(): data size = " << return_data.size() << endl;
+        }
         return_data_ = return_data;
         protocol_ = IOFetch::TCP;
         expected_ = IOFetch::SUCCESS;
@@ -489,27 +517,62 @@ TEST_F(IOFetchTest, TcpTimeout) {
     timeoutTest(IOFetch::TCP, tcp_fetch_);
 }
 
-// Do a send and return with a small amount of data
+// Test with values at or near 0, then at or near the chunk size (16 and 32
+// bytes, the sizes of the first two packets) then up to 65535.  These are done
+// in separate tests because in practice a new IOFetch is created for each
+// query/response exchange and we don't want to confuse matters in the test
+// by running the test with an IOFetch that has already done one exchange.
 
-TEST_F(IOFetchTest, TcpSendReceiveShort) {
-    tcpSendReturnTest(TEST_DATA);
+TEST_F(IOFetchTest, TcpSendReceive0) {
+    tcpSendReturnTest(test_data_.substr(0, 0));
 }
 
-// Now with at least 16kB of data.
+TEST_F(IOFetchTest, TcpSendReceive1) {
+    tcpSendReturnTest(test_data_.substr(0, 1));
+}
 
-TEST_F(IOFetchTest, TcpSendReceiveLong) {
-    const size_t REQUIRED_SIZE = 16 * 1024;
+TEST_F(IOFetchTest, TcpSendReceive15) {
+    tcpSendReturnTest(test_data_.substr(0, 15));
+}
 
-    // We could initialize the string with a repeat of a single character, but
-    // we choose to enure that there are different characters as an added test.
-    string data;
-    data.reserve(REQUIRED_SIZE);
-    while (data.size() < REQUIRED_SIZE) {
-        data += "An abritrary message that is returned to the IOFetch object!";
-    }
+TEST_F(IOFetchTest, TcpSendReceive16) {
+    tcpSendReturnTest(test_data_.substr(0, 16));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive17) {
+    tcpSendReturnTest(test_data_.substr(0, 17));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive31) {
+    tcpSendReturnTest(test_data_.substr(0, 31));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive32) {
+    tcpSendReturnTest(test_data_.substr(0, 32));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive33) {
+    tcpSendReturnTest(test_data_.substr(0, 33));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive4096) {
+    tcpSendReturnTest(test_data_.substr(0, 4096));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive8192) {
+    tcpSendReturnTest(test_data_.substr(0, 8192));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive16384) {
+    tcpSendReturnTest(test_data_.substr(0, 16384));
+}
+
+TEST_F(IOFetchTest, TcpSendReceive32768) {
+    tcpSendReturnTest(test_data_.substr(0, 32768));
+}
 
-    // ... and do the test with this data.
-    tcpSendReturnTest(data);
+TEST_F(IOFetchTest, TcpSendReceive65535) {
+    tcpSendReturnTest(test_data_.substr(0, 65535));
 }
 
 } // namespace asiolink




More information about the bind10-changes mailing list